gscam
gscam copied to clipboard
ros-drivers
Fixing camera_info_manager topic so that it conforms to the standard namespace
This bug made it into the releases, but should really be fixed.
According to the ROS camera spec, the camera info topic should be on camera/set_camera_info, but instead with gscam, it's just set_camera_info. This patch fixes that, but it might break code (mostly calibration scripts) for people who have been using gscam already.
While many older drivers still use the "camera" namespace, the camera prefix is not recommended practice any longer. See: REP-0135.
That REP clearly deprecates adding an "extra" level of namespace to the topic names. Services are not explicitly mentioned. I would still interpret them as working similarly.
In my opinion, this change breaks the API and should not be done in Hydro. Even changing it in Indigo should be done as a "tick-tock" update, with the old behavior supported but deprecated for an entire release cycle.
That is why camera1394 still does not comply with REP-0135. It's just too much hassle and too disruptive to make that kind of change.
Hm. So should the fix actually be to remove the camera sub-namespace from the other topics?
I don't know, haven't used this driver.
Removing the extra namespace would at least follow recommended practice, but it's tricky to do that correctly.
Either way, the compatibility issue seriously needs to be addressed. We don't want to put out a Hydro update that makes people's systems suddenly start failing.
It's an easy fix to remove the namespace, we just have to create two image transport publishers, and emit a warning when someone subscribed to the deprecated one.
Good idea.
That's probably a reasonable solution for deprecation in Indigo, with removal in J-turtle. I would not recommend changing Hydro.
I assume this driver is fairly widely used.
Good idea.
That's probably a reasonable solution for deprecation in Indigo, with removal in J-turtle. I would not recommend changing Hydro.
That sounds good. I'll update the PR accordingly.
I assume this driver is fairly widely used.
Yeah, even on OS X now! #15
Ok, @jack-oquin take a look at that. If it looks good, I'll branch hydro-devel out from the current master, merge this into master, and then rename master to indigo-devel
It's not for me to say. You are one of the maintainers. It looks OK to me.
Since you asked: :smile:
- I would only issue the deprecation warning once per subscription. Maybe that is what you are doing, I can't tell without reading the code in more detail.
- For my repositories, I develop in "master" and only create "hydro-devel" if I need to release a new change to Hydro after making Indigo changes like this. Until then, I have a release tag that permanently marks the latest Hydro version. But, if you anticipate releasing further Hydro changes, you may as well go ahead and branch now.
- Since there are API differences, the Indigo release should increment the minor number (to 0.2.0 in this case).
- Because I am a forgetful fellow, I'd also open an issue to remove the deprecated topic, with J-turtle the target.
Could we merge this PR please? I have multiple cameras and end up having to remap all the topics to remove the extra /camera namespace that's hard-coded.
