Add launch parameter to disable auto firmware update #61
Conversation
By default the Zivid ROS driver always updates the firmware of the camera. Some have expressed a desire to disable this behavior, so here we add a launch parameter that can disable this feature. Then the camera must be updated manually, fex in Zivid Studio or via ZividFirmwareUpdater.
| Or, if using `roslaunch` specify the parameter using `<param>`: | ||
|
|
||
| ```xml | ||
| <launch> | ||
| <node name="zivid_camera" pkg="zivid_camera" type="zivid_camera_node" ns="zivid_camera" output="screen"> | ||
| <param type="string" name="frame_id" value="camera_1" /> | ||
| </node> | ||
| </launch> |
There was a problem hiding this comment.
Nice to add the example of launching the zivid_camera node with parameters.
I think it would be nice to add the available launch parameters to the sample.launch file also, since it's most convenient to run the zivid_camera node (and the application code) from launch files. It would be nice to include the available options to the sample.launch file then.
There was a problem hiding this comment.
Maybe something like this:
<arg name="frame_id" default="zivid_optical_frame"/>
<node name="zivid_camera" pkg="zivid_camera" type="zivid_camera_node" ns="zivid_camera" output="screen">
<param type="string" name="frame_id" value="$(arg frame_id)"/>
</node>
There was a problem hiding this comment.
Then we can add most/all available options, like serial_number and update_firmware_automatically, to that sample launch file.
There was a problem hiding this comment.
Alternatively add a new zivid_camera launch file under https://github.com/zivid/zivid-ros/tree/master/zivid_camera/launch, and that launch file in the sample launch file.
There was a problem hiding this comment.
Yes, might be a good idea to add a launch file with all the parameters. Here I just wanted to add an example of how to set it for a .launch file yourself. The disadvantage of having all of them in the launch file is that we duplicate the default value, I suppose. We already have an existing request for this: #34, so I think we should not solve it now as part of this PR.
There was a problem hiding this comment.
There was a problem hiding this comment.
Feel free to make a PR for this @runenordmo
|
Looks good, added only one comment/suggestion 👍 |
apartridge
deleted the
add_launch_parameter_to_disable_auto_firmware_update
branch
No description provided.