Use serialnumber in camera names #101
Conversation
|
@reinzor @awesomebytes who is the best person to review this? |
|
@mikeferguson @130s could you maybe take a look at this? |
There was a problem hiding this comment.
- With a brief look, this looks good (i.e. esp. backward compatible). Can you confirm?
- Would you mind adding operational change if any in https://github.com/ros-drivers/openni2_camera/blob/noetic-devel/openni2_launch/doc/index.rst?
There was a problem hiding this comment.
Backwards compatibility looks correct to me - only suggestion is perhaps the ros parameter name should be more descriptive/clear - something like "use_serialnumber_as_name" (but maybe others thing name_serialnumber is descriptive enough - I don't feel strongly on this)
|
There was a problem hiding this comment.
Continues to look good to me - I'll give @130s a few days to review.
There was a problem hiding this comment.
- I don't think there are any changes that should be documented in file you suggested. I do have added it to the launch files (Which I cleaned a bit).
Well technically for those users who want to utilize the new options via launch file, there's an operational change (i.e. how to enable those options). However I guess a convention might be that those who are familiar enough with ROS to execute camera driver they can easily figure out the usage by looking at the out-most launch (openni2.launch). So I'm +1.
(I've increasingly dealt with experienced engineers who don't have prior knowledge about ROS. I fee like for some of those people these undocumented convention is still difficult)
|
And I think this is valuable to backport to prior releases (ROS Melodic, Kinetic). I'm on it now. After that I can release and trigger binary pkg generation. |
|
I think the noetic-devel branch can be used for the Melodic release. I don't know about Kinetic. |
|
Let's move the release conversation to #103 |
Fixes #100