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

Python DiagnosedPublisher crashes on publishing headerless topic #308

Open
JamesOwensMTC opened this issue Jun 28, 2023 · 2 comments
Open
Assignees
Labels
bug This is a bug in the code (and not a new feature) ros2 PR tackling a ROS2 branch

Comments

@JamesOwensMTC
Copy link

When publishing a headerless topic the node crashes with an AttributeError.

This is because:

  1. The publish method of DiagnosedPublisher always attempts to use the timestamp from a header to 'tick' the TopicDiagnostic
  2. The DiagnosedPublisher class in the python diagnostic_updater library initialises with a TopicDiagnostic task and never a HeaderlessTopicDiagnostic

The issue is within the following lines:

class DiagnosedPublisher(TopicDiagnostic):
"""
A TopicDiagnostic combined with a ros::Publisher.
For a standard ros::Publisher, this class allows the ros::Publisher and
the TopicDiagnostic to be combined for added convenience.
"""
def __init__(self, pub, diag, freq, stamp):
"""
Construct a DiagnosedPublisher.
@param pub The publisher on which statistics are being collected.
@param diag The diagnostic_updater that the CompositeDiagnosticTask
should add itself to.
@param freq The parameters for the FrequencyStatus class that will be
computing statistics.
@param stamp The parameters for the TimeStampStatus class that will be
computing statistics.
"""
TopicDiagnostic.__init__(self, pub.topic_name, diag, freq, stamp)
self.publisher = pub
def publish(self, message):
"""
Collect statistics and publishes the message.
The timestamp to be used by the TimeStampStatus class will be
extracted from message.header.stamp.
"""
self.tick(Time.from_msg(message.header.stamp))
self.publisher.publish(message)

See below a redacted example of the traceback when publishing a String message:

Traceback (most recent call last):
  File "/home/username/catkin_ws/src/package/nodes/node.py", line 90, in <module>
    test_node.run()
  File  "/home/username/catkin_ws/src/package/nodes/node.py", line 219, in run
    self.monitored_pub.publish(m)
  File "/opt/ros/melodic/lib/python2.7/dist-packages/diagnostic_updater/_publisher.py", line 143, in publish
    self.tick(message.header.stamp)
AttributeError: 'String' object has no attribute 'header'
[test_node-1] process has died [pid 17561, exit code 1, cmd /home/username/catkin_ws/src/package/nodes/node.py __name:=test_node __log:=/home/username/.ros/log/c92fbda4-158b-11ee-9322-080027d21e5a/test_node-1.log].
log file: /home/username/.ros/log/c92fbda4-158b-11ee-9322-080027d21e5a/test_node-1*.log

I experienced this issue using the library in ROS 1, but the same situation is evidently present in the ROS 2 code.

@ct2034
Copy link
Collaborator

ct2034 commented Jul 27, 2023

@JamesOwensMTC Thanks for reporting this. What is your suggestion to fix this?

@ct2034 ct2034 added bug This is a bug in the code (and not a new feature) ros2 PR tackling a ROS2 branch labels Jul 27, 2023
@ct2034 ct2034 self-assigned this Jul 27, 2023
@JamesOwensMTC
Copy link
Author

@ct2034 Following the pattern of the rest of this module a new class HeaderlessDiagnosedPublisher could be added.

This could look something like:

class HeaderlessDiagnosedPublisher(HeaderlessTopicDiagnostic):
    """
    A HeaderlessTopicDiagnostic combined with a ros::Publisher.

    For a standard ros::Publisher, this class allows the ros::Publisher and
    the HeaderlessTopicDiagnostic to be combined for added convenience.
    """

    def __init__(self, pub, diag, freq):
        """
        Construct a HeaderlessDiagnosedPublisher.

        @param pub The publisher on which statistics are being collected.
        @param diag The diagnostic_updater that the CompositeDiagnosticTask
        should add itself to.
        @param freq The parameters for the FrequencyStatus class that will be
        computing statistics.
        """
        HeaderlessTopicDiagnostic.__init__(self, pub.topic_name, diag, freq)
        self.publisher = pub

    def publish(self, message):
        """
        Collect statistics and publishes the message.
        """
        self.tick()
        self.publisher.publish(message)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is a bug in the code (and not a new feature) ros2 PR tackling a ROS2 branch
Projects
None yet
Development

No branches or pull requests

2 participants