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

ntrip_client should support ZED-F9P for applications other than mav-ros - i.e. support KumarRobotics driver #10

Closed
PaulBouchier opened this issue Apr 2, 2022 · 4 comments · Fixed by #22 or #21

Comments

@PaulBouchier
Copy link

This is an enhancement request, with an embedded question.

This package is the only ntrip client I found that runs on ROS noetic. I see questions in issues on other packages like some of the 256 forks of ntrip_ros :-( about running on noetic vs. melodic (Python 3 vs 2). I think this package could be more broadly applicable than to just the mav-ros env. It looks to me like the most mature ZED-F9P driver package is Kumar Robotics package (which supports all the ublox receivers), but it uses rtcm_msgs/Message for RTCM data, whereas this package uses mavros_msgs/RTCM for RTCM data. The two message definitions are identical. I get that this package is probably intended for the UAV community, where mav is really big. But there's a big non-UAV autonomous vehicle community out there and I think they (and I) need a version of a noetic ntrip client that works with KumarRobotics driver. I made your package work by substituting rtcm_msgs/Message for mavros_msgs/RTCM - it worked fine.

The question: what SW do you expect to publish the nmea_msgs/Sentence topic? I made my own talker node that publishes a hard-coded string. Many of the ntrip_ros packages require a parameter containing a nmea GGA string that they forward to the ntrip server. They even point the user to a web page that will generate the GGA string. That's reasonably flexible. How do you expect it to be done?

Enhancement ideas: I've considered some alternatives for enhancing this package, listed below, but haven't found any that I think are acceptable. Do you have any thoughts?

  1. Add a parameter that lets the user select whether rtcm data should be published as mavros_msgs/RTCM or ntrip_msgs/rtcm. Disadvantage: you don't want mav users to have to install rtcm_msgs, nor general users to have to install mavros_msgs. Is there a way to make the message import happen only for the parameter-selected message type? E.g. put the import inside a conditional check. If I can make that work would you consider a pull request?
@robbiefish
Copy link
Contributor

Hi @PaulBouchier

Thanks for your thorough write up on this issue.

The question: what SW do you expect to publish the nmea_msgs/Sentence topic?

The NMEA message is only required when connecting to a "network" NTRIP caster so that it can direct the client to the best NTRIP caster. If you are connecting to a non-network NTRIP caster, you do not need to publish the NMEA message. This client was designed to work with our microstrain_inertial ROS node which publishes NMEA sentences.

I get that this package is probably intended for the UAV community, where mav is really big. But there's a big non-UAV autonomous vehicle community out there and I think they (and I) need a version of a noetic ntrip client that works with KumarRobotics driver.

Although this driver should work for the UAV community, ideally it shouldn't matter what package you want to use this node with. I just chose the mavros RTCM message because it was the first thing I found when I was looking for a predefined RTCM message.

Is there a way to make the message import happen only for the parameter-selected message type? E.g. put the import inside a conditional check. If I can make that work would you consider a pull request?

Because this node is implemented in python, I think this is doable as you could put a try except around the imports, set a flag if the imports succeed, and then check that flag along with a config parameter to determine which messages are published. We do something similar to determine the ROS version in another package. I would be more than happy to review a PR with this feature.

@PaulBouchier
Copy link
Author

Thanks for your support Rob. I implemented what you suggested - a parameter to select RTCM message type (mavros_msgs/RTCM or rtcm_msgs/Message), and it sets flags if either message type can't be imported and throws an exception if the parameter selects a type that couldn't be imported. It is at https://github.com/PaulBouchier/ntrip_client/tree/rtcm_msgs_support (in branch rtcm_msgs_support). However there's a problem and I'd appreciate your input on what to do.

The problem is what to put in package.xml & CMakeLists.txt. The current design declares mavros_msgs as a dependency, and catkin_make fails if mavros_msgs is not installed (as it should). But should I add rtcm_msgs as a dependency? It is one if the user intends to use that message. That would force all users of the current version to apt install ros-noetic-rtcm-msgs, regardless of the fact that current SW only uses mavros_msgs. Or I could leave it not declared as a dependency, and document the need to install it in the README.md, which is not good practice. What do you think?

@robbiefish
Copy link
Contributor

Sorry it took me awhile to get back to you on this. I think that the problem here is what to put in the package.xml. Since the rtcm_msgs package only appears to be available on ROS, I think that it would be best to just leave the package.xml as it is, so that it works the same way for ROS and ROS2, and then document that for ROS, users can install the rtcm_msgs package to switch the message type. I realize this is probably not best practice because it will require non-mavros users to install the mavros_msgs package, but I think it is better to have as much consistency between ROS and ROS2 as possible. If the rtcm_msgs package is ported to ROS2, we can reassess this.

I think that you could add something like this to the CMakeLists.txt to allow either one to be used when the package is being built:

# Find either of the supported RTCM messages
set(MAVROS_MSGS_PKG mavros_msgs)
set(RTCM_MSGS_PKG rtcm_msgs)
find_package(${MAVROS_MSGS_PKG})
find_package(${RTCM_MSGS_PKG})
if(${MAVROS_MSGS_PKG}_FOUND)
  list(APPEND RTCM_DEPS ${MAVROS_MSGS_PKG})
endif()
if(${RTCM_MSGS_PKG}_FOUND)
  list(APPEND RTCM_DEPS ${RTCM_MSGS_PKG})
endif()
if(NOT ${MAVROS_MSGS_PKG}_FOUND AND NOT ${RTCM_MSGS_PKG}_FOUND)
  message(FATAL_ERROR "Unable to find either ${MAVROS_MSGS_PKG} or ${RTCM_MSGS_PKG}. At least one must be insalled on the system")
endif()

...

catkin_package(
  CATKIN_DEPENDS
    rospy
    std_msgs
    nmea_msgs
    ${RTCM_DEPS}
)

I haven't actually tested the above code, but according to this ROS answers it should work

@rikba
Copy link

rikba commented May 20, 2022

Maybe an alternative solution in the meantime:
We use topic_tools transform to generate the rtcm_msgs/Message from mavros_msgs/RTCM.

https://github.com/ethz-asl/ublox_utils/blob/main/launch/ublox.launch#L72-L74

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