Skip to content

Commit

Permalink
pw_sensor: Allow missing 'org' in compatible descriptor
Browse files Browse the repository at this point in the history
When providing a sensor descriptor, most will leverage the org,part
schema. This does not always seem to be the case. There are times where
the org is missing when used in downstream Zephyr. We should allow for
this case and only include the org when provided and not empty.

BUG=b/386143823

Change-Id: Ie3f044b5adbbcbc76ed13716c705561e87adac32

Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/256773
Lint: Lint 🤖 <[email protected]>
Commit-Queue: Yuval Peress <[email protected]>
Reviewed-by: Carlos Chinchilla <[email protected]>
Docs-Not-Needed: Yuval Peress <[email protected]>
  • Loading branch information
yperess authored and CQ Bot Account committed Jan 16, 2025
1 parent 2558be0 commit fdfca20
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 8 deletions.
1 change: 0 additions & 1 deletion pw_sensor/py/pw_sensor/metadata_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
},
"additionalProperties": false,
"required": [
"org",
"part"
]
},
Expand Down
3 changes: 1 addition & 2 deletions pw_sensor/py/pw_sensor/resolved_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
"type": "object",
"description": "Set of sensors using their compatible string as a key in the format 'org','part'",
"patternProperties": {
"^[a-zA-Z_][a-zA-Z0-9_\\-]*,[a-zA-Z_][a-zA-Z0-9_\\-]*$": {
"^[a-zA-Z0-9_\\-]*,?[a-zA-Z_][a-zA-Z0-9_\\-]*$": {
"type": "object",
"description": "A single sensor definition",
"properties": {
Expand All @@ -140,7 +140,6 @@
},
"additionalProperties": false,
"required": [
"org",
"part"
]
},
Expand Down
45 changes: 41 additions & 4 deletions pw_sensor/py/pw_sensor/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,15 @@ def validate(self, metadata: dict) -> dict:
# Resolve all trigger entries
self._resolve_triggers(metadata=metadata, out=result)

compatible = metadata.pop("compatible")
compatible, compatible_str = Validator._get_compatible_string_and_dict(
metadata.pop("compatible")
)
supported_buses = metadata.pop("supported-buses")
channels = metadata.pop("channels")
attributes = metadata.pop("attributes")
triggers = metadata.pop("triggers")

result["sensors"][f"{compatible['org']},{compatible['part']}"] = {
result["sensors"][compatible_str] = {
"compatible": compatible,
"supported-buses": self._normalize_supported_buses(supported_buses),
"channels": channels,
Expand All @@ -152,10 +154,11 @@ def validate(self, metadata: dict) -> dict:
try:
jsonschema.validate(instance=result, schema=_RESOLVED_SCHEMA)
except jsonschema.exceptions.ValidationError as e:
raise RuntimeError(
msg = (
"ERROR: Malformed output YAML: "
f"{yaml.safe_dump(result, indent=2)}"
) from e
)
raise RuntimeError(msg) from e

return result

Expand Down Expand Up @@ -185,6 +188,40 @@ def _normalize_supported_buses(buses: list[str]) -> list[str]:
raise RuntimeError(error)
return filtered_list

@staticmethod
def _get_compatible_string_and_dict(
compatible: dict[str, str],
) -> tuple[dict[str, str], str]:
"""
Normalize compatible info
This function processes a 'compatible' dictionary with a 'part' key and
an optional 'org' key. It returns a new dictionary with the 'org' key
removed if it was empty or missing, and a formatted string based on the
'org' key's presence and value.
Args:
compatible (dict[str, str]): A dictionary with a 'part' key and an
optional 'org' key.
Returns:
Tuple[dict[str, str], str]: A tuple containing:
- A new dictionary with the 'org' key removed if it was empty or
missing.
- A formatted string:
- "{org},{part}" if 'org' exists and is not empty (after trimming)
- "part" otherwise.
"""
part = compatible["part"].lower()
org = compatible.get("org", "").strip().lower()

new_compatible = {"part": part}
if org:
new_compatible["org"] = org
return new_compatible, f"{org},{part}"
return new_compatible, part

def _resolve_dependencies(self, metadata: dict, out: dict) -> None:
"""
Given a list of dependencies, ensure that each of them exists and
Expand Down
42 changes: 41 additions & 1 deletion pw_sensor/py/validator_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def test_invalid_compatible_type(self) -> None:
+ "supported-buses:\n- i2c"
),
cause_substrings=[
"'org' is a required property",
"'part' is a required property",
],
)

Expand Down Expand Up @@ -73,6 +73,46 @@ def test_invalid_compatible_type(self) -> None:
cause_substrings=[" is not of type 'object'"],
)

def test_partial_compatible_string(self) -> None:
"""
Check that missing 'org' generates correct keys and empty entries are
removed.
"""
metadata: dict = {
"compatible": {"part": "pigweed"},
"supported-buses": ["i2c"],
}
result = Validator().validate(metadata=metadata)
self.assertIn("pigweed", result["sensors"])
self.assertDictEqual(
{"part": "pigweed"},
result["sensors"]["pigweed"]["compatible"],
)

metadata["compatible"]["org"] = " "
result = Validator().validate(metadata=metadata)
self.assertIn("pigweed", result["sensors"])
self.assertDictEqual(
{"part": "pigweed"},
result["sensors"]["pigweed"]["compatible"],
)

def test_compatible_string_to_lower(self) -> None:
"""
Check that compatible components are converted to lowercase and
stripped.
"""
metadata = {
"compatible": {"org": "Google", "part": "Pigweed"},
"supported-buses": ["i2c"],
}
result = Validator().validate(metadata=metadata)
self.assertIn("google,pigweed", result["sensors"])
self.assertDictEqual(
{"org": "google", "part": "pigweed"},
result["sensors"]["google,pigweed"]["compatible"],
)

def test_invalid_supported_buses(self) -> None:
"""
Check that invalid or missing supported-buses cause an error
Expand Down

0 comments on commit fdfca20

Please sign in to comment.