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

video: emul: virtual driver for an imager and RX engine #79482

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

Conversation

josuah
Copy link
Collaborator

@josuah josuah commented Oct 6, 2024

This takes inspiration on the other skeleton.c drivers through Zephyr tree:
https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/usb/udc/udc_skeleton.c
https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/display/display_dummy.c

It acts as a starting point and as a puppet to explain API changes affecting image sensors.

Be used for providing test cases for the entire API, as required for stable APIs.

Utilities are introduced in the process to reduce boilerplate.

Twister command (new with this PR):

west twister --inline-logs --platform native_sim --scenario tests/drivers/video/api/drivers.video.api

Fixes: #73867

@josuah
Copy link
Collaborator Author

josuah commented Oct 6, 2024

I note that the frame interval API was merged, this will need to include it.

@josuah josuah added DNM This PR should not be merged (Do Not Merge) and removed DNM This PR should not be merged (Do Not Merge) labels Oct 6, 2024
@josuah josuah marked this pull request as draft October 6, 2024 23:23
@josuah josuah force-pushed the pr-video-sensor-skeleton branch 5 times, most recently from eb4a7a4 to cfa3bfb Compare October 13, 2024 13:20
@josuah josuah marked this pull request as ready for review October 13, 2024 13:20
@zephyrbot zephyrbot added the area: Base OS Base OS Library (lib/os) label Oct 13, 2024
@josuah
Copy link
Collaborator Author

josuah commented Oct 13, 2024

  • Aims to cover the most frequent use-cases and facilitate modifications on top
  • Assume that image sensor drivers are apply modes described by pixelformat+resolution+framerate
  • For every new sensor, require filling a few tables and let the ready-made functions hook them to the Zephyr video API

@josuah josuah requested review from pillo79 and uLipe October 13, 2024 13:36
@josuah josuah force-pushed the pr-video-sensor-skeleton branch 2 times, most recently from 64e0235 to 9a1a450 Compare October 14, 2024 12:02
@zephyrbot zephyrbot added platform: STM32 ST Micro STM32 area: Watchdog Watchdog labels Oct 14, 2024
@josuah josuah force-pushed the pr-video-sensor-skeleton branch 2 times, most recently from 12321fb to 9d5cd84 Compare October 23, 2024 09:13
@josuah
Copy link
Collaborator Author

josuah commented Oct 23, 2024

force-push: unit tests for the emulated drivers: this covers the entire video API except set_signal()

To test: west twister --inline-logs --platform native_sim --scenario tests/drivers/video/api/drivers.video.api

@ngphibang
Copy link
Contributor

@josuah : Is the PR ready for review ? Sorry, I asked because I received email notifications of the continuous force-push rather often that make me thinking that this is in development phase. If so, is it better to limit the force push not only to avoid email notifications but also to reduce Zephyr CI kicked-off burden for every push ? If it's not the case, it's OK to do that.

@josuah
Copy link
Collaborator Author

josuah commented Oct 23, 2024

@ngphibang you are right, I should work on a separate wip branch and force-push there instead of directly to the PR branch. Thank you for letting me know. Yes this is ready. Twister, check_compliance, and doc build worked locally.

[EDIT: this was thought as being ready at first, and then I thought "let's add unit tests to the same PR", and then "lets' add some more", etc: I should take some time to let things mature and be more reasonable]

@josuah josuah removed the DNM This PR should not be merged (Do Not Merge) label Oct 23, 2024
@josuah
Copy link
Collaborator Author

josuah commented Nov 2, 2024

force-push:

  • Fix 2 commits being mixed-up
  • Fix bindings and deviceetree (include i2c-device.yaml or base.yaml)
  • Use devicetree macros API like in drivers: video: Add endpoint DT helpers #80649
  • Move video-common.h to video.h as the helpers are usable by the application too.

@josuah
Copy link
Collaborator Author

josuah commented Nov 3, 2024

force-push:

josuah added a commit to tinyvision-ai-inc/zephyr that referenced this pull request Nov 4, 2024
PR zephyrproject-rtos#79482 is where this commit would be added

Signed-off-by: Josuah Demangeon <[email protected]>
josuah added a commit to tinyvision-ai-inc/zephyr that referenced this pull request Nov 4, 2024
PR zephyrproject-rtos#79482 is where this commit would be added

Signed-off-by: Josuah Demangeon <[email protected]>
josuah added a commit to tinyvision-ai-inc/zephyr that referenced this pull request Nov 4, 2024
PR zephyrproject-rtos#79482 is where this commit would be added

Signed-off-by: Josuah Demangeon <[email protected]>
josuah added a commit to tinyvision-ai-inc/zephyr that referenced this pull request Nov 12, 2024
PR zephyrproject-rtos#79482 is where this commit would be added

Signed-off-by: Josuah Demangeon <[email protected]>
josuah added a commit to tinyvision-ai-inc/zephyr that referenced this pull request Nov 12, 2024
PR zephyrproject-rtos#79482 is where this commit would be added

Signed-off-by: Josuah Demangeon <[email protected]>
josuah added a commit to tinyvision-ai-inc/zephyr that referenced this pull request Nov 17, 2024
PR zephyrproject-rtos#79482 is where this commit would be added

Signed-off-by: Josuah Demangeon <[email protected]>
josuah added a commit to tinyvision-ai-inc/zephyr that referenced this pull request Nov 21, 2024
PR zephyrproject-rtos#79482 is where this commit would be added

Signed-off-by: Josuah Demangeon <[email protected]>
@josuah
Copy link
Collaborator Author

josuah commented Nov 27, 2024

In the process of implementing these emulated drivers, I introduced more video API helpers to reduce boilerplate.

If introducing helpers does not make sense in this same PR, I can integrate them back into the emulated drivers to reduce the amount of things this introduces.

@ngphibang
Copy link
Contributor

I will try to review this whenever possible, thanks !

return emul_imager_write_reg(dev, EMUL_IMAGER_REG_CTRL, 0);
}

static const struct video_driver_api emul_imager_driver_api = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use the DEVICE_API wrapper.

Suggested change
static const struct video_driver_api emul_imager_driver_api = {
static DEVICE_API(video, emul_imager_driver_api) = {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes thank you, done.
This will be helpful for checking device types in the video shell based on this PR!

return 0;
}

static const struct video_driver_api emul_rx_driver_api = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
static const struct video_driver_api emul_rx_driver_api = {
static DEVICE_API(video, emul_rx_driver_api) = {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done as well, thank you for the reminder.

*
* @return The frame interval value in microseconds.
*/
uint64_t video_frmival_nsec(const struct video_frmival *frmival);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this be a simple inline helper?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I agree. Done now.

Introduce a video_get_format_index() utility to help finding a caps
entry out of a given format. Introduce several utilities to seek and
apply frame intervals.

Signed-off-by: Josuah Demangeon <[email protected]>
Add a new implementation of a test pattern generator, with the same
architecture as real drivers: split receiver core and
I2C-controlled sub-device, with changes of video format in
"zephyr,emul-imager" leads to different data produced by
"zephyr,emul-rx".

Signed-off-by: Josuah Demangeon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

drivers: video: sensors: add a sensor_skeleton.c to speed-up contribution of simple sensors
8 participants