sunnylink: dynamic param metadata (#1522)

* feat(params): add support for parameter metadata retrieval

- Introduced `getKeyMetadata` method for accessing metadata associated with params.
- Enhanced `getParamsAllKeysV1` to include metadata parsing and optional dynamic enum generation.
- Extended unit tests to verify metadata parsing, enum mapping, and edge cases.

* Revert "feat(params): add support for parameter metadata retrieval"

This reverts commit 865b695ff966bfab7dab4d211fb2d91f06fe92db.

* update: integrate params metadata management and unit tests

- Added `update_params_metadata.py` to manage and update parameters metadata.
- Enhanced `getParamsAllKeysV1` to include metadata for params.
- Created comprehensive tests (`test_params_metadata.py`, `test_params_sync.py`) to validate metadata integrity and params consistency.

* update: improve params metadata readability and enhance enums

- Renamed params titles for clarity and consistency.
- Added enum options and mappings to selected params for better usability.

* update: enhance params metadata with improved enum structures

- Replaced plain enum lists with detailed objects (`value`, `label`) for clarity.
- Standardized parameter options for consistency across metadata.

* update: add validation constraints to params metadata

- Introduced `min`, `max`, and `step` attributes for improved parameter range validation.
- Enhances user input handling and ensures consistency in metadata.

* lint

* more lint stuff and permissions

* does this suffice?

* more lint

* update: refine params type hinting and remove unused shebang

- Adjusted type annotation in `params_dict` for better compatibility.
- Removed unnecessary shebang from `test_params_metadata.py`.

* update: expand test coverage for params metadata validation

- Added detailed test cases to ensure metadata consistency (`options`, `constraints`, `titles`).
- Validates API response alignment with `params_metadata.json`.

* the finals

* names

---------

Co-authored-by: Jason Wen <haibin.wen3@gmail.com>

sunnypilot/sunnylink/athena/sunnylinkd.py

@@ -1,5 +1,10 @@
 #!/usr/bin/env python3
+"""
+Copyright (c) 2021-, Haibin Wen, sunnypilot, and a number of other contributors.
 
+This file is part of sunnypilot and is licensed under the MIT License.
+See the LICENSE.md file in the root directory for more details.
+"""
 from __future__ import annotations
 
 import base64
@@ -32,9 +37,11 @@ LOCAL_PORT_WHITELIST = {8022}
 SUNNYLINK_LOG_ATTR_NAME = "user.sunny.upload"
 SUNNYLINK_RECONNECT_TIMEOUT_S = 70  # FYI changing this will also would require a change on sidebar.cc
 DISALLOW_LOG_UPLOAD = threading.Event()
+METADATA_PATH = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "params_metadata.json")
 
 params = Params()
 
+
 def handle_long_poll(ws: WebSocket, exit_event: threading.Event | None) -> None:
   cloudlog.info("sunnylinkd.handle_long_poll started")
   sm = messaging.SubMaster(['deviceState'])
@@ -180,16 +187,30 @@ def getParamsAllKeys() -> list[str]:
 
 @dispatcher.add_method
 def getParamsAllKeysV1() -> dict[str, str]:
+  try:
+    with open(METADATA_PATH) as f:
+      metadata = json.load(f)
+  except Exception:
+    cloudlog.exception("sunnylinkd.getParamsAllKeysV1.exception")
+    metadata = {}
+
   available_keys: list[str] = [k.decode('utf-8') for k in Params().all_keys()]
 
-  params_dict: dict[str, list[dict[str, str | bool | int | None]]] = {"params": []}
+  params_dict: dict[str, list[dict[str, str | bool | int | object | dict | None]]] = {"params": []}
   for key in available_keys:
     value = get_param_as_byte(key, get_default=True)
-    params_dict["params"].append({
+
+    param_entry = {
       "key": key,
       "type": int(params.get_type(key).value),
       "default_value": base64.b64encode(value).decode('utf-8') if value else None,
-    })
+    }
+
+    if key in metadata:
+      meta_copy = metadata[key].copy()
+      param_entry["_extra"] = meta_copy
+
+    params_dict["params"].append(param_entry)
 
   return {"keys": json.dumps(params_dict.get("params", []))}
 
@@ -238,10 +259,7 @@ def startLocalProxy(global_end_event: threading.Event, remote_ws_uri: str, local
 
   cloudlog.debug("athena.startLocalProxy.starting")
   ws = create_connection(
-    remote_ws_uri,
-    header={"Authorization": f"Bearer {sunnylink_api.get_token()}"},
-    enable_multithread=True,
-    sslopt={"cert_reqs": ssl.CERT_NONE}
+    remote_ws_uri, header={"Authorization": f"Bearer {sunnylink_api.get_token()}"}, enable_multithread=True, sslopt={"cert_reqs": ssl.CERT_NONE}
   )
 
   return start_local_proxy_shim(global_end_event, local_port, ws)

sunnypilot/sunnylink/tests/test_params_metadata.py

@@ -0,0 +1,86 @@
+"""
+Copyright (c) 2021-, Haibin Wen, sunnypilot, and a number of other contributors.
+
+This file is part of sunnypilot and is licensed under the MIT License.
+See the LICENSE.md file in the root directory for more details.
+"""
+import json
+
+from openpilot.sunnypilot.sunnylink.athena.sunnylinkd import getParamsAllKeysV1, METADATA_PATH
+
+
+def test_get_params_all_keys_v1():
+  """
+  Test the getParamsAllKeysV1 API endpoint.
+
+  Why:
+  This endpoint is used by the UI (and potentially external tools) to fetch the list of
+  available parameters along with their metadata (titles, descriptions, options, constraints).
+  We need to ensure it returns the correct structure and that the metadata from
+  params_metadata.json is correctly merged into the response.
+
+  Expected:
+  - The response should contain a "keys" field which is a JSON string of a list of parameters.
+  - Each parameter object should have "key", "type", "default_value", and optionally "_extra".
+  - The "_extra" field should contain the rich metadata (title, options, min/max, etc.) matching
+    the source of truth (params_metadata.json).
+  """
+  response = getParamsAllKeysV1()
+  assert "keys" in response
+
+  keys_json = response["keys"]
+  params_list = json.loads(keys_json)
+
+  assert isinstance(params_list, list)
+  assert len(params_list) > 0
+
+  # Check structure of first item
+  first_param = params_list[0]
+  assert "key" in first_param
+  assert "type" in first_param
+  assert "default_value" in first_param
+
+  if "_extra" in first_param:
+    assert isinstance(first_param["_extra"], dict)
+    assert "default" not in first_param["_extra"]
+    assert "type" not in first_param["_extra"]
+
+  # Load the source of truth
+  with open(METADATA_PATH) as f:
+    metadata = json.load(f)
+
+  # Verify that the API response matches the metadata file for a few sample keys
+  # This ensures the plumbing is working without being brittle to content changes
+
+  # 1. Check a key that should have metadata
+  keys_with_metadata = [k for k in params_list if k["key"] in metadata]
+  assert len(keys_with_metadata) > 0, "No parameters found that match metadata keys"
+
+  for param in keys_with_metadata[:5]:  # Check first 5 matches
+    key = param["key"]
+    expected_meta = metadata[key]
+
+    assert "_extra" in param, f"Parameter {key} should have _extra field"
+    actual_meta = param["_extra"]
+
+    # Verify all fields in JSON are present in the API response
+    for meta_key, meta_val in expected_meta.items():
+      assert meta_key in actual_meta, f"Missing {meta_key} in API response for {key}"
+      assert actual_meta[meta_key] == meta_val, f"Mismatch for {key}.{meta_key}: expected {meta_val}, got {actual_meta[meta_key]}"
+
+  # 2. Check that we are correctly serving options if they exist
+  params_with_options = [k for k in keys_with_metadata if "options" in k.get("_extra", {})]
+  if params_with_options:
+    param = params_with_options[0]
+    key = param["key"]
+    assert isinstance(param["_extra"]["options"], list), f"Options for {key} should be a list"
+    assert param["_extra"]["options"] == metadata[key]["options"]
+
+  # 3. Check that we are correctly serving numeric constraints if they exist
+  params_with_constraints = [k for k in keys_with_metadata if "min" in k.get("_extra", {})]
+  if params_with_constraints:
+    param = params_with_constraints[0]
+    key = param["key"]
+    assert param["_extra"]["min"] == metadata[key]["min"]
+    assert param["_extra"]["max"] == metadata[key]["max"]
+    assert param["_extra"]["step"] == metadata[key]["step"]

sunnypilot/sunnylink/tests/test_params_sync.py

@@ -0,0 +1,202 @@
+"""
+Copyright (c) 2021-, Haibin Wen, sunnypilot, and a number of other contributors.
+
+This file is part of sunnypilot and is licensed under the MIT License.
+See the LICENSE.md file in the root directory for more details.
+"""
+import json
+import os
+import pytest
+
+from openpilot.common.params import Params
+from openpilot.sunnypilot.sunnylink.athena.sunnylinkd import METADATA_PATH
+
+
+def test_metadata_json_exists():
+  """
+  Test that the params_metadata.json file exists at the expected path.
+
+  Why:
+  The metadata file is the source of truth for parameter descriptions, options, and constraints.
+  If it's missing, the UI will not be able to display rich information for parameters.
+
+  Expected:
+  The file should exist at sunnypilot/sunnylink/params_metadata.json.
+  """
+  assert os.path.exists(METADATA_PATH), f"Metadata file not found at {METADATA_PATH}"
+
+
+def test_metadata_json_valid():
+  """
+  Test that the params_metadata.json file contains valid JSON.
+
+  Why:
+  Invalid JSON will cause the metadata loading to fail, potentially crashing the UI or
+  resulting in missing metadata.
+
+  Expected:
+  The file content should be parseable as a JSON object (dictionary).
+  """
+  with open(METADATA_PATH) as f:
+    try:
+      data = json.load(f)
+    except json.JSONDecodeError:
+      pytest.fail("Metadata file is not valid JSON")
+
+    assert isinstance(data, dict), "Metadata root must be a dictionary"
+
+
+def test_all_params_have_metadata():
+  """
+  Test that every parameter in the codebase has a corresponding entry in params_metadata.json.
+
+  Why:
+  We want to ensure 100% coverage of parameter metadata. Any parameter added to the codebase
+  should also be documented in the metadata file.
+
+  Expected:
+  There should be no parameters in Params() that are missing from the metadata file.
+  If this fails, run 'python3 sunnypilot/sunnylink/tools/update_params_metadata.py'.
+  """
+  params = Params()
+  all_keys = [k.decode('utf-8') for k in params.all_keys()]
+
+  with open(METADATA_PATH) as f:
+    metadata = json.load(f)
+
+  missing_keys = [key for key in all_keys if key not in metadata]
+
+  if missing_keys:
+    pytest.fail(
+      f"The following parameters are missing from metadata: {missing_keys}. "
+      + "Please run 'python3 sunnypilot/sunnylink/tools/update_params_metadata.py' to update."
+    )
+
+
+def test_metadata_keys_exist_in_params():
+  """
+  Test that all keys in params_metadata.json actually exist in the codebase.
+
+  Why:
+  We want to avoid stale metadata for parameters that have been removed or renamed.
+  This keeps the metadata file clean and relevant.
+
+  Expected:
+  There should be no keys in the metadata file that are not present in Params().
+  This prints a warning rather than failing, as it's less critical than missing metadata.
+  """
+  params = Params()
+  all_keys = {k.decode('utf-8') for k in params.all_keys()}
+
+  with open(METADATA_PATH) as f:
+    metadata = json.load(f)
+
+  extra_keys = [key for key in metadata.keys() if key not in all_keys]
+
+  if extra_keys:
+    print(f"Warning: The following keys in metadata do not exist in Params: {extra_keys}")
+
+
+def test_no_default_titles():
+  """
+  Test that no parameter has a title that is identical to its key.
+
+  Why:
+  The default behavior of the update script is to set the title equal to the key.
+  We want to force developers to provide human-readable, descriptive titles for all parameters.
+
+  Expected:
+  No parameter metadata should have 'title' == 'key'.
+  """
+  with open(METADATA_PATH) as f:
+    metadata = json.load(f)
+
+  default_title_keys = [key for key, meta in metadata.items() if meta.get("title") == key]
+
+  if default_title_keys:
+    pytest.fail(
+      f"The following parameters have default titles (title == key): {default_title_keys}. "
+      + "Please update 'params_metadata.json' with descriptive titles."
+    )
+
+
+def test_options_structure():
+  """
+  Test that the 'options' field in metadata follows the correct structure.
+
+  Why:
+  The UI expects 'options' to be a list of objects with 'value' and 'label' keys.
+  Incorrect structure will break the UI rendering for dropdowns/toggles.
+
+  Expected:
+  If 'options' is present, it must be a list of dicts, and each dict must have 'value' and 'label'.
+  """
+  with open(METADATA_PATH) as f:
+    metadata = json.load(f)
+
+  for key, meta in metadata.items():
+    if "options" in meta:
+      options = meta["options"]
+      assert isinstance(options, list), f"Options for {key} must be a list"
+      for option in options:
+        assert isinstance(option, dict), f"Option in {key} must be a dictionary"
+        assert "value" in option, f"Option in {key} must have a 'value' key"
+        assert "label" in option, f"Option in {key} must have a 'label' key"
+
+
+def test_numeric_constraints():
+  """
+  Test that numeric parameters have valid 'min', 'max', and 'step' constraints.
+
+  Why:
+  The UI uses these constraints to validate user input and render sliders/steppers.
+  Missing or invalid constraints can lead to UI bugs or invalid parameter values.
+
+  Expected:
+  If any of min/max/step is present, ALL of them must be present.
+  They must be numbers (int/float), and min must be less than max.
+  """
+  with open(METADATA_PATH) as f:
+    metadata = json.load(f)
+
+  for key, meta in metadata.items():
+    if "min" in meta or "max" in meta or "step" in meta:
+      assert "min" in meta, f"Numeric param {key} must have 'min'"
+      assert "max" in meta, f"Numeric param {key} must have 'max'"
+      assert "step" in meta, f"Numeric param {key} must have 'step'"
+
+      assert isinstance(meta["min"], (int, float)), f"Min for {key} must be number"
+      assert isinstance(meta["max"], (int, float)), f"Max for {key} must be number"
+      assert isinstance(meta["step"], (int, float)), f"Step for {key} must be number"
+      assert meta["min"] < meta["max"], f"Min must be less than max for {key}"
+
+
+def test_known_params_metadata():
+  """
+  Test specific known parameters to ensure they have the expected rich metadata.
+
+  Why:
+  This acts as a spot check to ensure that our rich metadata population logic is working correctly
+  and that critical parameters (like LongitudinalPersonality) have their options and constraints preserved.
+
+  Expected:
+  'LongitudinalPersonality' should have 3 options (Aggressive, Standard, Relaxed).
+  'CustomAccLongPressIncrement' should have min=1, max=10, step=1.
+  """
+  with open(METADATA_PATH) as f:
+    metadata = json.load(f)
+
+  # Check an enum-like param
+  lp = metadata.get("LongitudinalPersonality")
+  assert lp is not None
+  assert "options" in lp
+  assert len(lp["options"]) == 3
+  assert lp["options"][0]["label"] == "Aggressive"
+  assert lp["options"][0]["value"] == 0
+
+  # Check a numeric param
+  acc_long = metadata.get("CustomAccLongPressIncrement")
+  assert acc_long is not None
+  assert acc_long["min"] == 1
+  assert acc_long["max"] == 10
+  assert acc_long["step"] == 1

sunnypilot/sunnylink/tools/update_params_metadata.py

@@ -0,0 +1,56 @@
+#!/usr/bin/env python3
+"""
+Copyright (c) 2021-, Haibin Wen, sunnypilot, and a number of other contributors.
+
+This file is part of sunnypilot and is licensed under the MIT License.
+See the LICENSE.md file in the root directory for more details.
+"""
+import json
+import os
+
+from openpilot.common.params import Params
+
+METADATA_PATH = os.path.join(os.path.dirname(__file__), "../params_metadata.json")
+
+
+def main():
+  params = Params()
+  all_keys = params.all_keys()
+
+  if os.path.exists(METADATA_PATH):
+    with open(METADATA_PATH) as f:
+      try:
+        data = json.load(f)
+      except json.JSONDecodeError:
+        data = {}
+  else:
+    data = {}
+
+  # Add new keys
+  for key in all_keys:
+    key_str = key.decode("utf-8")
+    if key_str not in data:
+      print(f"Adding new key: {key_str}")
+      data[key_str] = {
+        "title": key_str,
+        "description": "",
+      }
+
+  # Remove deleted keys
+  # keys_to_remove = [k for k in data.keys() if k.encode("utf-8") not in all_keys]
+  # for k in keys_to_remove:
+  #   print(f"Removing deleted key: {k}")
+  #   del data[k]
+
+  # Sort keys
+  sorted_data = dict(sorted(data.items()))
+
+  with open(METADATA_PATH, "w") as f:
+    json.dump(sorted_data, f, indent=2)
+    f.write("\n")
+
+  print(f"Updated {METADATA_PATH}")
+
+
+if __name__ == "__main__":
+  main()