Devicetree DTS and YAML format improvements

[KenVanHoeylandt]

Summary by CodeRabbit

• New Features

  • Binding properties now support default values.

• Improvements

  • Compiler returns meaningful exit codes and reports errors more clearly.
  • Stronger validation of device configurations with unified error handling.

• Changes

  • Platform binding schemas updated: some fields made required, others gained explicit defaults.
  • Many device-tree files simplified by removing unused/placeholder pin and transfer-size entries.

• Tests

  • Added integration tests and a dedicated Devicetree test workflow.

• Documentation

  • Removed several outdated TODO items.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Introduces a new DevicetreeException and replaces many generic Exception raises with it across transformer, generator, binding discovery, and parsing code. Adds an optional default field to BindingProperty and propagates YAML default values via the parser. Changes main to return an int and catches DevicetreeException; compile.py now exits with sys.exit(result). Updates PlatformEsp32 binding YAMLs, adds test data and integration tests, and adds a CI job DevicetreeTests.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Devicetree DTS and YAML format improvements' accurately summarizes the primary changes: updates to devicetree compiler code (exception handling, exit codes, default values), YAML binding files (added required fields, default values), and DTS device files (removed optional properties with defaults).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dts-improvements-2

---

No actionable comments were generated in the recent review. 🎉

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (3)

Buildscripts/DevicetreeCompiler/source/transformer.py (1)

5-7: Consider explicit imports from source.models instead of star import.

The static analysis tool flags DeviceProperty (line 51) as potentially undefined due to the wildcard import on line 5. An explicit import would silence this and improve readability.

-from source.models import *
+from source.models import DtsVersion, Device, DeviceProperty, PropertyValue, IncludeC, DefineC

Buildscripts/DevicetreeCompiler/source/generator.py (2)

62-77: "int" and "value" branches are identical — consider consolidating.

Both type == "value" (line 64) and type == "int" (line 66) return property.value unchanged. If the semantic distinction isn't needed elsewhere, they could share a branch to reduce duplication.

Proposed consolidation

-    if type == "value":
-        return property.value
-    elif type == "int":
+    if type in ("value", "int"):
         return property.value

---

103-120: Non-required property without a default raises an error — is this intentional?

Line 118 raises DevicetreeException for a non-required property that has no default. This effectively means every binding property must either be required, have a default, or be of type "bool". If a property is truly optional with no sensible default (e.g., an optional label), the current logic provides no way to omit it — the pre-allocated result array (line 102) assumes every binding property maps to a config struct field.

If this is by design (binding authors must always specify defaults for optional properties), a brief comment documenting this invariant would help future maintainers.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

Buildscripts/DevicetreeCompiler/source/generator.py (1)

62-75: ⚠️ Potential issue | 🔴 Critical

",".join(property.value) will raise TypeError when a phandle appears in a values property.

The grammar allows values: VALUE+, where each VALUE can be a NUMBER, PHANDLE, or C_VARIABLE. The PHANDLE handler returns a PropertyValue object while others return strings. When a values property contains a phandle (e.g., < &device_alias 10 >), the resulting property.value is a list mixing PropertyValue objects and strings. Since PropertyValue has no __str__ method, calling ",".join(property.value) will fail with TypeError: expected str instance, PropertyValue found. The code should extract string representations of all elements before joining, such as ",".join(item.value if isinstance(item, PropertyValue) else str(item) for item in property.value).

🧹 Nitpick comments (1)

Buildscripts/DevicetreeCompiler/source/main.py (1)

51-53: Bare except DevicetreeException won't catch other failure modes.

Only DevicetreeException is caught here, but the code inside the try block can also raise FileNotFoundError (from read_file), lark.exceptions.LarkError (from parsing), OSError, etc. These will still produce unhandled tracebacks. Consider adding a broader fallback except Exception to print a clean error for unexpected failures too.

Proposed fix

     except DevicetreeException as caught:
         print("\033[31mError: ", caught, "\033[0m")
         return 1
+    except Exception as caught:
+        print("\033[31mUnexpected error: ", caught, "\033[0m")
+        return 1

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

.github/workflows/tests.yml (1)

37-39: Consider pinning dependency versions for reproducible CI builds.

Unpinned pip install lark pyyaml can break CI silently when a new major version ships an incompatible change.

Proposed fix

-        run: pip install lark pyyaml
+        run: pip install lark==1.2.2 pyyaml==6.0.2

Adjust versions to match what the project currently uses.

Buildscripts/DevicetreeCompiler/tests/test_integration.py (1)

14-21: Add a timeout to subprocess.run to prevent tests from hanging indefinitely.

If the compiler enters an infinite loop or hangs on malformed input, this test will block the CI pipeline forever.

Proposed fix

 def run_compiler(config_path, output_path):
     result = subprocess.run(
         [sys.executable, COMPILE_SCRIPT, config_path, output_path],
         capture_output=True,
         text=True,
-        cwd=PROJECT_ROOT
+        cwd=PROJECT_ROOT,
+        timeout=60
     )
     return result