search

autowarefoundation / ros2_socketcan

A ROS2 wrapper around Linux SocketCAN
Apache License 2.0
117 stars 60 forks source link

Naming of can_tx and can_rx #4

Closed kenji-miyake closed 3 years ago

kenji-miyake commented 3 years ago

Currently, this package uses can_tx for sending topics(receiving can data) and can_rx for receiving topics(sending can data).

I guess this is from kvaser_interface. https://github.com/astuff/kvaser_interface/blob/56b65758814a6d56ad04a6008334514d84123a8c/src/kvaser_reader_node.cpp#L66 https://github.com/astuff/kvaser_interface/blob/56b65758814a6d56ad04a6008334514d84123a8c/src/kvaser_writer_node.cpp#L68

However, I feel this causes a bit of confusion because reader_node uses tx and writer_node uses rx. So I think it's better to use can_tx for sending can data(receiving topics) and can_rx for receiving can data(sending topics).

For example, ros-industrial's socketcan_bridge uses this rule. (Naming is a bit different, rx -> received, tx -> sent) https://github.com/ros-industrial/socketcan_interface/blob/4d105e6eac6b36acd285c00641a25dbd315eb58a/socketcan_bridge/src/topic_to_socketcan.cpp#L37 https://github.com/ros-industrial/socketcan_interface/blob/4d105e6eac6b36acd285c00641a25dbd315eb58a/socketcan_bridge/src/socketcan_to_topic.cpp#L51

@JWhitleyWork Could you tell me your thoughts?

JWhitleyWork commented 3 years ago

Yeah, I did copy this from kvaser_interface. However, the naming depends on whether you're looking at the data from the point of view of the ROS user or from the sensor. If you're coming from the sensor's point of view, sent would be data being sent from the sensor and received would be data received by the sensor. However, if you're looking at it from the point of view of the ROS user, sent would be data sent to the sensor while received would be data received from the sensor. While I agree that can_tx and can_rx might be backward, I still think they could be more precise.

To make it completely unambiguous, how do you feel about to_can and from_can or to_bus and from_bus?

kenji-miyake commented 3 years ago

@JWhitleyWork I agree with you. For the people who are familiar with CAN, tx and rx are fine, but others can't understand easily. In addition, tx and rx cause the point-of-view problem, so I think there are better namings.

To make it completely unambiguous, how do you feel about to_can and from_can or to_bus and from_bus?

I think to_can and fron_can are ambiguous because the ROS message is can_msgs. to_bus and from_bus (or to_can_bus and from_can_bus?) seem good to me!

JWhitleyWork commented 3 years ago

Changed to to_can_bus and from_can_bus in 468719f.