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

ROS2 Reliable QoS Support #48

Closed
MazFed opened this issue Jan 10, 2024 · 7 comments · Fixed by #61
Closed

ROS2 Reliable QoS Support #48

MazFed opened this issue Jan 10, 2024 · 7 comments · Fixed by #61

Comments

@MazFed
Copy link

MazFed commented Jan 10, 2024

Hello,

I'm trying to use the mqtt_client to subscribe to ROS2 topics with QoS of type RELIABLE, VOLATILE but i get the error of incompatible QoS:

[mqtt_client-1] [WARN] [1704882384.623365432] [mqtt_client]: New publisher discovered on topic '/laser/data', offering incompatible QoS. No messages will be sent to it. Last incompatible policy: RELIABILITY_QOS_POLICY

I saw in the documentation that it is possible to change the QoS of MQTT topics. Is it possible to do the same thing for ROS2 topics as well?

@lreiher
Copy link
Member

lreiher commented Jan 17, 2024

Currently, the mqtt_client doesn't give precise control over the QoS settings of publishers and subscribers. This is most likely due to coming from ROS 1, where one only had to specify a queue_size.

Note that the queue_size is still configurable in the ROS 2 version as well:

Subscription

ros2mqtt.ros.subscriber = create_generic_subscription(
          ros_topic, msg_type, ros2mqtt.ros.queue_size, bound_callback_func);

Publisher

mqtt2ros.ros.publisher = create_generic_publisher(
        mqtt2ros.ros.topic, ros_msg_type, mqtt2ros.ros.queue_size);

These functions actually require to pass an rclcpp::QoS object (doc), but I belive that passing an integer implicitly uses a special rclcpp::QoS constructor that only sets history_depth (doc).

I guess it would make sense to add more configuration options to mqtt_client to make the QoS profile fully configurable. Please feel free to work on that, I'd be happy to merge a PR! Unfortunately, I will not have time for this anytime soon, but could still assist with suggestions on how to implement.

@MazFed
Copy link
Author

MazFed commented Jan 25, 2024

Hi @lreiher, thanks for the explanation.
Indeed when an integer is passed, it is used to set history_depth of QoS object with "transient local" durability and "reliable" reliability.
For my test configuration i only need to serialize topics that have "best effort" reliability.
So for now i just made a temporary modification replacing the QoS object passed to the create_generic_subscription function to one compatible, like this:

rclcpp::QoS m_qosBestEffort = rclcpp::QoS(ros2mqtt.ros.queue_size)
    .reliability(RMW_QOS_POLICY_RELIABILITY_BEST_EFFORT);

ros2mqtt.ros.subscriber = create_generic_subscription(
    ros_topic, msg_type, m_qosBestEffort, bound_callback_func);

I noticed that this also guarantees me compatibility with the "reliable" topics, although I suppose it doesn't guarantee me reliability.
In case I need to use mixed types of QoS, I will try to make a more complete and elegant implementation to submit.

@JayHerpin
Copy link
Contributor

I think it make sense to have explicit QoS support for the mqtt->ros converison, but for the ros -> mqtt conversion since the subscribers are created lazily, isn't it as simple as doing: get_publisher_info_by_topic instead of get_topic_names_and_types and then using the QoS matching the publisher?

@JayHerpin
Copy link
Contributor

JayHerpin commented Apr 16, 2024

at the risk of hijacking, I am working on a patch locally that adds configuration like this:

mqtt2ros:
 ...
   my/mqtt/topic:
          ros_topic: /my/ros/topic
          ros_type: std_msgs/msg/Bool
          advanced:
            ros:
               queue_size: 1
               qos:
                  reliability: reliable
                  durability: transient_local

its not just setting the QoS, its being explicit with the ros message type creation (explicitly about the mqtt->ros direction)

But I am not sure that doing something similar in the other direction is really useful since it seems fine to me to just wait until we see a publisher and get the type and qos information from them.

@lreiher
Copy link
Member

lreiher commented Apr 18, 2024

Cool that you are working on flexibly defining QoS!

For the ros-to-mqtt connections, how about also offering the flexible configuration, but also allowing to the QoS parameters to auto? In the auto case you would simply infer the information from the publisher.

@JayHerpin
Copy link
Contributor

sure, I could do that. I did implement that auto qos fix I suggested and it does work. I don't mind adding in the explicit for that direction flow direction as well.

@lreiher
Copy link
Member

lreiher commented Apr 18, 2024

Sounds great, looking forward to a PR!

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