-
Notifications
You must be signed in to change notification settings - Fork 58
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
SocketCAN filters #25
SocketCAN filters #25
Conversation
61b6db4
to
9e52660
Compare
@MarcelDudek Thank you for your proposal. Since the maintenance resource is very limited, we need your support. @JWhitleyWork cc @xmfcx @mitsudome-r Do you have any comments on this PR? We still need your help. Thank you. |
23b48f5
to
f464206
Compare
@kenji-miyake I've updated the pull request description to be much more detailed and made some changes to the code:
As for more specific example usage: Launching the receiver node with filters $ ros2 launch ros2_socketcan socket_can_receiver.launch.py filters:=[0x0,0x1,0x101,0x7FF] interface:=vcan0
[INFO] [launch]: All log files can be found below /home/marcel/.ros/log/2022-10-22-21-05-44-239848-marcel-ubuntu-55654
[INFO] [launch]: Default logging verbosity is set to INFO
[INFO] [socket_can_receiver_node_exe-1]: process started with pid [55666]
[socket_can_receiver_node_exe-1] [INFO] [1666465544.351602102] [socket_can_receiver]: interface: vcan0
[socket_can_receiver_node_exe-1] [INFO] [1666465544.351655036] [socket_can_receiver]: interval(s): 0.010000
[socket_can_receiver_node_exe-1] [INFO] [1666465544.351665452] [socket_can_receiver]: use bus time: 0
[socket_can_receiver_node_exe-1] [INFO] [1666465544.351702268] [socket_can_receiver]: filters [id:mask]: [0:1],[101:7ff] this will pass all following CAN frames $ cansend vcan0 020#DEADBEEF
$ cansend vcan0 346#DEADBEEF
$ cansend vcan0 70A#DEADBEEF
$ cansend vcan0 101#DEADBEEF but will block $ cansend vcan0 333#DEADBEEF
$ cansend vcan0 103#DEADBEEF
$ cansend vcan0 70B#DEADBEEF so on ROS topic only 4 messages are passed $ ros2 topic echo /from_can_bus
header:
stamp:
sec: 1666465766
nanosec: 496604220
frame_id: can
id: 32
is_rtr: false
is_extended: false
is_error: false
dlc: 4
data:
- 222
- 173
- 190
- 239
- 0
- 0
- 0
- 0
---
header:
stamp:
sec: 1666465776
nanosec: 796864702
frame_id: can
id: 838
is_rtr: false
is_extended: false
is_error: false
dlc: 4
data:
- 222
- 173
- 190
- 239
- 0
- 0
- 0
- 0
---
header:
stamp:
sec: 1666465789
nanosec: 30715724
frame_id: can
id: 1802
is_rtr: false
is_extended: false
is_error: false
dlc: 4
data:
- 222
- 173
- 190
- 239
- 0
- 0
- 0
- 0
---
header:
stamp:
sec: 1666465791
nanosec: 808752690
frame_id: can
id: 257
is_rtr: false
is_extended: false
is_error: false
dlc: 4
data:
- 222
- 173
- 190
- 239
- 0
- 0
- 0
- 0
--- By default, SocketCAN uses filter |
f464206
to
1514016
Compare
Hi @kenji-miyake @JWhitleyWork! I've redone filtering code. It now supports a full range of SocketCAN filtering options. Filters are now passed to node via string. Filters are formatted exactly the same as in candump. See updated pull request description for more information. From code side, I've added |
556f6ad
to
395e574
Compare
Hi @kenji-miyake @JWhitleyWork. I fixed a problem with tests failures. On my machine, I hadn't had flake8-quotes installed, so no error had been reported. Anyway, I fixed the problem with one line in the launch file, so all tests will now hopefully pass. By the way, do you want me to squash both commits into one? I've kept them separate, so you can see the difference between the current and previous implementation. |
Thank you for your work.
@MarcelDudek I think you don't need this because it's automatically done when we merge a PR. |
In a system with multiple CAN devices, the ROS nodes interacting with API each want to set up their own filters because they are listening for different PGN's. Is the design intent here that the ROS2 architect combines all the ROS2 filters together and each ROS consumer then gets extra data at each consuming node, or the designer spawns unique socket can nodes for each consumer, or some other method? Example: I have a wheel angle sensor, an IMU, and GPS. Each have their own PGN's.
What is the recommendation for what I run using this library with the associated filters. |
@RFRIEDM-Trimble To be honest, both solutions are viable. Filters are set up per socket, so it really is your decision if you want to have multiple sockets with different filters, or if you want to have one socket and some data will be redundant for each of the node. I think in terms of performance, multiple sockets might be a bit better, because filtering is done at the kernel level instead of the node application level. And you reduce communication on the topics. But I haven't done any performance tests in such configuration, so I don't know. |
Good to know. For some of the CAN heavy applications with multiple CAN busses, and many CAN applications, it would start to matter. With the bus loads I've seen, there have been problems with putting all data on the same topic. A third approach that can be evaluated that's now supported in Humble is the use of DDS content filtering, which could combine the best of both worlds by filtering by PGN for example. |
Hi @kenji-miyake @JWhitleyWork @xmfcx. Are you able to review this request in upcoming days? I'd like to finish it, if possible. |
@MarcelDudek Sorry for my late responses. Regarding code, since most all of the code was written by @JWhitleyWork, it's okay for me (and maybe also for the AWF) if he approves. The only thing I'd like to ask you is whether the following filtering format follows some standards.
If it does, please write some reference links in the description. If it doesn't, what made you design in such a way? ( I guess my question above is just because I don't have enough knowledge to review this PR properly. |
@kenji-miyake No problem 😄
Yes it does. The syntax for filters, as I stated in the description, this is a syntax of canutil's candump. Most of the people working with socketcan's filters from terminal should be familiar with it. Having it the same as candump makes it also possible to test filters in terminal and then apply them to the node. Here is the documentation of candump from which I took the description of filters. I also added this link to the description. So then, I wait for the review from @JWhitleyWork when possible. Thanks. |
@MarcelDudek Thank you for your quick reply! |
@kenji-miyake Yeah, I can add it in the comments, no problem. Though I don't see any readme file for this repo 😅 Do you mean |
@MarcelDudek Oh, sorry. I misunderstood that there was a README. (I'm not so familiar with this repo yet. 🥺 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it. Nice feature addition!
@xmfcx @mitsudome-r I'll approve and merge this after a reply about comments from @MarcelDudek. Please let me know if there is any issue. |
Filters can be set using launch parameter. A list of pairs (can id and mask) are fetched and applied to socket filter. Added unit test for filters application and fuctionality. Signed-off-by: Marcel Dudek <lotnik1998@gmail.com>
SocketCAN filters can be now with string description used by candump utility. Added support for error masks and joined CAN filters. Instead of list of integers, receiver node now uses string parameter in order to receive filters. Filters will now be parsed and setup during configuration. Added unit test for parsing and updated filters unit test. Signed-off-by: Marcel Dudek <lotnik1998@gmail.com>
Added links referencing man-pages docs for candump, containing more information about socketcan filters syntax used. Links were added to doxygen documentation of filters parsing method and to launch argument description. Signed-off-by: Marcel Dudek <lotnik1998@gmail.com>
395e574
to
1039dcf
Compare
@kenji-miyake Hi! I've rebased the branch on current main and I've added comments that you had asekd for with commit 1039dcf. Link to man-pages was added to:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work, and thank you for addressing our comments!
CAN filters support.
A list of SocketCAN filters can be applied to the receiver. This allows for software filtering of unwanted messages on the socket level, reducing communication overhead when applicable.
Filters can be applied to receiver node using ROS2 parameter.
filters
parameter accepts a string of filters which is parsed by the receiver and applied on configuration of the node. By default, socket_can_receiver.launch.py will apply a filter which accepts all data frames, though it can be changed usingfilters
launch argument.Filters
Full support for all 3 socket options of filtering (
CAN_RAW_FILTER
,CAN_RAW_ERR_FILTER
andCAN_RAW_JOIN_FILTERS
) has been added. Filter encoding is exactly the same as for canutils' candump:Examples of filters
123:C00007FF
will pass only frames with id 0x123 of standard length. RTR frames will not be passed.123:3FFFFFFF
will pass standard and extended frames with id 0x123. It will also pass RTR frames of this id.0:1
will pass only frames with even id (SFF, EFF and RTR).400:FFFFFF00
will pass standard frames with id 0x400-0x499.0:0
will pass only all frames.0~0,#FFFFFFFF
will pass none of data fames, but will pass all error frames.400:FFFFFF00,0:1
will pass all standard frames with if id 0x400-0x499 and all frames (extended and remote inlcuded) of even id.400:FFFFFF00,0:1,j
will pass only standard frames with even id from 0x400-0x499 (e.g. 0x402 will pass but 0x403 will not)Usage
Using launch system:
$ ros2 launch ros2_socketcan socket_can_receiver.launch.py interface:=vcan0 filters:=101:7FF,103:7FF,0:1 [INFO] [launch]: All log files can be found below /home/marcel/.ros/log/2022-10-28-21-58-31-844506-marcel-ubuntu-55568 [INFO] [launch]: Default logging verbosity is set to INFO [INFO] [socket_can_receiver_node_exe-1]: process started with pid [55580] [socket_can_receiver_node_exe-1] [INFO] [1666987111.956802116] [socket_can_receiver]: interface: vcan0 [socket_can_receiver_node_exe-1] [INFO] [1666987111.956856888] [socket_can_receiver]: interval(s): 0.010000 [socket_can_receiver_node_exe-1] [INFO] [1666987111.956871573] [socket_can_receiver]: use bus time: 0 [socket_can_receiver_node_exe-1] [INFO] [1666987112.191972338] [socket_can_receiver]: applied filters: 101:7FF,103:7FF,0:1
Using run: