Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sensors: codegen for attr/chan/trig #83077

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

yperess
Copy link
Collaborator

@yperess yperess commented Dec 17, 2024

Introduce phase 1 of the sensor code generation. In this phase, we flatten the sensor attribute/channel/trigger enums such that sensors which add their own private values will not collide with others. Downstream users can add their own sensor descriptors by modifying various cmake variables: ZEPHYR_EXTRA_SENSOR_YAML_FILES, ZEPHYR_EXTRA_SENSOR_INCLUDE_PATHS, and ZEPHYR_EXTRA_SENSOR_DEFINITION_FILES.

Note:

We're still working on adding the rest of the sensor.yaml files.

Why is CI failing?

CI doesn't allow new python dependencies to be added in the same PR that they're used. Instead we would need to first check in the python dependencies, then roll out a new version of the docker, then re-run this PR's CI.

Why not devicetree?

When we started off, devicetree was very heavily explored. 2 approaches were taken with devicetree.

  1. Adding default values to bindings.
  2. Adding child nodes to devicetree sensor nodes.

Details about devicetree limitations for sensors can be found at #71235. While this PR doesn't yet solve the exact issue in the issue, the additional metadata can/will be added once the basic descriptors are added.

Timeline

This is an MVP for the sensor.yaml which allows us feature parity with today + a small improvement where we're able to avoid custom attribute/channel/trigger collision. Along with the ability to statically assert that a sensor driver supports the features we need. The next steps are (keep in mind that some of these may happen in parallel):

  1. Generate documentation. See sensor: codegen #81748 as an example. This documentation models after the board catalog and allows us to quickly search for sensor drivers matching some criteria.
  2. Generate driver libraries. Looking at the adi sensors introduced by Analog Devices Inc we can see a great pattern emerging on how to abstract the bus, model the RTIO submit calls, and attribute handling. A lot of that information can be pulled from the sensor.yaml and we can generate a fairly comprehensive {compatible.part}.h and {compatible.part}.c which would handle the majority of the driver functions while leaving out a few stubs to be added by the developer. For example, this could look like (in cmake):
zephyr_sensor_driver(
  YAML
    sensor.yaml
  SRCS
    rtio.c
    decoder.c
    $<$<BOOL:${CONFIG_MY_SENSOR_I2C}>:i2c.c>
    $<$<BOOL:${CONFIG_MY_SENSOR_SPI}>:spi.c>
    $<$<BOOL:${CONFIG_MY_SENSOR_STREAMING}>:streaming.c>
)
  1. Adding size information. Here we would introduce size information for both internal storage and decoded. This would allow us to go back to the second step and auto populate the buffers.
  2. Add support to west manifests to be able to list additional sensor.yaml specs which would allow modules to append to the ZEPHYR_EXTRA_SENSOR_* cmake variables
  3. Generate tests. Since sensors now explicitly say "I support reading channel X", we can generate tests for that. Assuming there's a fully implemented emulator, we should be able to verify that a driver actually implements what it says.

Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmake indent is 2 spaces, apply comments throughout whole PR

cmake/modules/extensions.cmake Outdated Show resolved Hide resolved
cmake/modules/extensions.cmake Outdated Show resolved Hide resolved
${arg_INPUTS}
${arg_SOURCES}
)
add_custom_target(${NAME}.__generate_constants
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are there full stops in target names?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not? I've used them before in other projects. I'm happy to remove them. I use them to denote subpackage like targets. Makes it harder for things to collide when generating targets.

I'm happy to remove the . if you feel strongly about it.

Comment on lines +28 to +29
# Used to generate the sensor catalog
pigweed>=0.0.20
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does every user need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Pigweed python library is where we developed the sensor codegen library. This allowed us to start onboarding teams that don't use Zephyr to use the same sensor standard descriptor without having to add Zephyr as a dependency (yet).

scripts/requirements-base.txt Show resolved Hide resolved
cmake/modules/extensions.cmake Outdated Show resolved Hide resolved
cmake/modules/extensions.cmake Outdated Show resolved Hide resolved
Without these version requirements, existing older sphinx
installations satisfy the requirement but fail to build.

Signed-off-by: Yuval Peress <[email protected]>
Add the Pigweed python dependency used to generate the sensor catalog

Signed-off-by: Yuval Peress <[email protected]>
Add the Pigweed python dependency used to generate sensor headers

Signed-off-by: Yuval Peress <[email protected]>
Allow users to generate their own sensor libraries from downstream
sensor YAML descriptors.

Signed-off-by: Yuval Peress <[email protected]>
Signed-off-by: Al Semjonovs <[email protected]>
Add code generation for the sensor attribute, channel, and trigger
enums. Included in this is the ability to query the sensor's
information statically using the compatible string similar to how
devicetree does it.

Signed-off-by: Yuval Peress <[email protected]>
@yperess yperess force-pushed the peress/sensor-c-codegen branch from 6fc1215 to 354b954 Compare January 9, 2025 19:20
@yperess yperess requested a review from nordicjm January 9, 2025 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants