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

Add support for command nodes + Oryx parameter file #163

Merged
merged 4 commits into from
Mar 25, 2024

Conversation

shtrophic
Copy link
Contributor

@shtrophic shtrophic commented Mar 22, 2024

Currently, parameter definition files do not support declaring command nodes.
I suggest adding this to the camera driver, as some commands are useful to be executed during camera operation.

This would enable, for example, online sequencer configuration or single-frame acquisition via enumeration node AcquisitionMode: SingleFrame and command node AcqusitionStart that is then executed.

As I see it now, an issue is that executing these nodes would require passing a value, even though there should be none required. (cp. ros2 param set camera_driver acquisition_start foo, foo being unnecessary.)

@berndpfrommer
Copy link
Collaborator

berndpfrommer commented Mar 22, 2024

So the idea is that you trigger executions by setting a parameter, rather than using the standard ROS service mechanism? A bit unconventional but it allows for a uniform implementation. And one can always implement a service call mechanism alongside later, if desired.
Question: have you tested that you can repeatedly trigger an execution by setting the parameter repeatedly (to the same value)? If you had to change the (fake) value every time to effect a callback, that would be kinda tedious on the caller's side.
Request: add an example execution node to one of the camera yaml config files.
Needs some fixes to the formatting. Use "colcon test --packages-select spinnaker_camera_driver" to verify before committing.
Other than that LGTM.

@shtrophic
Copy link
Contributor Author

Question: have you tested that you can repeatedly trigger an execution by setting the parameter repeatedly (to the same value)? If you had to change the (fake) value every time to effect a callback, that would be kinda tedious on the caller's side.

Here a small demonstration via software-triggering:

cast.webm

Request: add an example execution node to one of the camera yaml config files.

I only have Oryx cameras on site where I can verify correct node names. (AcquisitionControl/TriggerSoftware should also exist on Blackfly cameras but I cannot verify that.)

Other than that, something like this is enough:

- name: trigger_software
  type: command
  node: AcquisitionControl/TriggerSoftware

Unrelated: how complete do you expect a camera parameter file to be? I am asking since as mentioned I am working with some Oryx cameras where I needed parameters that were not available in the provided .yaml files.

@shtrophic shtrophic marked this pull request as ready for review March 25, 2024 12:17
@berndpfrommer
Copy link
Collaborator

Any camera config file is better than none! People are much more willing to experiment if they have a file to start with. Please by all means commit your oryx.yaml file with this PR. If you don't get error messages on startup, that's good enough. Remove parameters that you copy-and-pasted from e.g. blackfly and that you are not setting, i.e. that are untested and may throw errors if used.

If people need more parameters than you, they should figure out how to define them. Not your job.

@shtrophic shtrophic changed the title Add support for command nodes Add support for command nodes + Oryx parameter file Mar 25, 2024
@shtrophic
Copy link
Contributor Author

Sure, I added it to the PR. I can't say it is production ready though.
Especially for my use case which is sequencer configuration, I right now rather run a script after launching the node for setting the parameters, instead of passing the parameters during launch.

Example for two sequences with different exposure times
#!/bin/bash

node=$1

function setp {
	( set -x; ros2 param set $node $1 $2 )
}

set -e

setp sequencer_mode "'Off'"

setp sequencer_feature_selector ExposureTime
setp sequencer_feature_enable True

setp sequencer_feature_selector Gain
setp sequencer_feature_enable False

setp sequencer_configuration_mode "'On'"

setp sequencer_set_selector 0
setp sequencer_feature_selector ExposureTime
setp exposure_auto "'Off'"
setp exposure_mode Timed
setp exposure_time 5000.0
setp sequencer_trigger_source FrameStart
setp sequencer_trigger_activation RisingEdge
setp sequencer_set_next 1
setp sequencer_set_save "_"

setp sequencer_set_selector 1
setp exposure_time 30000.0
setp sequencer_trigger_source FrameStart
setp sequencer_trigger_activation RisingEdge
setp sequencer_set_next 0
setp sequencer_set_save "_"

setp sequencer_set_start 0
setp sequencer_set_selector 0
setp sequencer_set_load "_"
setp sequencer_configuration_mode "'Off'"

setp sequencer_mode "'On'"

@berndpfrommer berndpfrommer merged commit 9b63c19 into ros-drivers:humble-devel Mar 25, 2024
2 checks passed
@berndpfrommer
Copy link
Collaborator

I am planning on rewriting the documentation for the driver. Will add this new feature to the docs then. @Sir-Photch thanks again for your contribution!

@shtrophic shtrophic deleted the commandnode branch March 25, 2024 19:56
@shtrophic
Copy link
Contributor Author

You are welcome. Will this change also land in iron? I just based this on humble-devel since it was indicated as the default branch

@berndpfrommer
Copy link
Collaborator

berndpfrommer commented Mar 25, 2024 via email

@berndpfrommer
Copy link
Collaborator

berndpfrommer commented Mar 28, 2024 via email

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.

2 participants