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

WIP: Add tests and update params logic #242

Closed

Conversation

flynneva
Copy link
Collaborator

@flynneva flynneva commented Apr 8, 2023

  • add more tests, fix bugs found
  • reorganize parameters logic

@flynneva flynneva force-pushed the add-tests-and-update-params-logic branch 4 times, most recently from f0657d8 to d3ebd99 Compare April 9, 2023 02:31
@flynneva flynneva force-pushed the add-tests-and-update-params-logic branch 15 times, most recently from 4d38f30 to 58548e6 Compare April 10, 2023 12:10
@@ -314,6 +320,79 @@ class UsbCam
return m_image.pixel_format;
}

/// @brief Send current parameters to V4L2 device
/// TODO(flynneva): only send parameters that changed
inline void set_v4l2_params()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are the inline functions really needed here? Maybe, it is better to implement the classic prototype + body model? For the function called only once it is not ever needed, why we should keep this [useless] specifier here? I guess, it is inherited from the legacy and we can remove the inline specifiers without any problem. This is only my opinion, so please share what you think about any real reason we have here


TEST_F(test_usb_cam_lib_fixture, usb_cam_class_basic) {
// Ensure parameters were properly set using some helper functions
ASSERT_EQ(m_test_cam->get_image_width(), size_t(480));
Copy link
Collaborator

Choose a reason for hiding this comment

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

480x640 or 640x480? Is the frame orientation inverted?

@twdragon
Copy link
Collaborator

https://github.com/ros-drivers/usb_cam/actions/runs/4657295606/jobs/8245890280?pr=242#step:5:1883

Run cd /home/runner/work/_actions/flynneva/renode-linux-runner-action/disable-multicore-emulation
  cd /home/runner/work/_actions/flynneva/renode-linux-runner-action/disable-multicore-emulation
  sudo python3 action/run-in-renode.py "ls /dev | grep video
  " "vivid"
  shell: bash --noprofile --norc -e -o pipefail {0}
python3: can't open file 'action/run-in-renode.py': [Errno 2] No such file

@flynneva it seems one script from the internal CI setup is missed

@flynneva
Copy link
Collaborator Author

@twdragon yeah this is still a WIP....sorry I forgot to mention that

@flynneva flynneva changed the title Add tests and update params logic WIP: Add tests and update params logic Apr 15, 2023
@flynneva
Copy link
Collaborator Author

@twdragon im close to getting a virtual V4l2 device up and running in CI, meaning we can do more integration testing on each PR

@flynneva flynneva force-pushed the add-tests-and-update-params-logic branch 6 times, most recently from 923533f to b3f2af3 Compare April 30, 2023 22:37
@flynneva flynneva force-pushed the add-tests-and-update-params-logic branch 6 times, most recently from 071053d to 4b35cf9 Compare May 6, 2023 22:48
@flynneva flynneva force-pushed the add-tests-and-update-params-logic branch 5 times, most recently from c9e5d23 to 37c34c9 Compare May 20, 2023 15:44
@flynneva flynneva force-pushed the add-tests-and-update-params-logic branch 6 times, most recently from c34b20f to 5d63d95 Compare June 6, 2023 04:32
@flynneva flynneva force-pushed the add-tests-and-update-params-logic branch from 5d63d95 to bcab08e Compare June 6, 2023 04:36
@flynneva
Copy link
Collaborator Author

flynneva commented Jun 6, 2023

Going to close this MR until renode supports more architecdtures. Otherwise we'd have to recompile ROS 2 for RISC-V 🙃 Not that I dont want to...just I dont think I have time to make that work 😅

@flynneva flynneva closed this Jun 6, 2023
@mgielda
Copy link

mgielda commented Sep 29, 2023

Hi @flynneva note that Renode does support many architectures, including wide support for ARM (see some updates in https://antmicro.com/blog/2023/09/renode-1-14-release/). Of course we need to update the action to feature all the new archs, but armv7-a is possible now, and armv8-a is coming soon (and we can make this "sooner" if there is a need!)

See: https://github.com/antmicro/renode-linux-runner-action#architecture

@mgielda
Copy link

mgielda commented Sep 29, 2023

Please also shoot me an email at [email protected], happy to chat more!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants