Repository navigation
Install a CAN_RAW_FILTER for the registered receive ids - #178
Open
pradeepsuryad wants to merge 3 commits into
Open
pradeepsuryad wants to merge 3 commits into
pradeepsuryad wants to merge 3 commits into
Conversation
CANSocket received every frame on the interface: replies from motors it did not register, and the command frames of any other process on the same bus, which SocketCAN loops back by default. diagnose --explain and the health example read those as unmatched frames, a sign of a misconfigured master id, and they used up recv_all()'s 64-frame budget alongside this process's own replies. CANSocket now keeps a set of receive filters, through add_recv_filter() and remove_recv_filter(). They are counted, so the collections sharing a socket can each add the same one. CANDeviceCollection adds a filter for each device it holds and removes it with the device. Until the first filter is added, and again after the last one is removed, the socket receives everything as before. Error frames come through CAN_RAW_ERR_FILTER and are not affected. Fixes enactic#165.
A motor whose master id (RID 7) was never set replies on 0x00. No device registers that id, so with the receive filters those replies no longer reach the socket, and the unmatched-frame hint in diagnose --explain and in the health example could never fire. Both now add a filter for 0x00.
Until the first device is registered the socket receives everything, and what arrives in that window is still queued once the filter is in place. The first recv_all() then counted it as unmatched, which on a busy bus is the same false warning the filters are there to stop. When the first filter goes in, empty the queue first. Error frames in it are still recorded in the bus status.
|
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #165.
This follows the proposal in #165, in three commits.
1. Install a CAN_RAW_FILTER for the registered receive ids.
CANSocketgetsadd_recv_filter(can_id, mask)andremove_recv_filter(can_id, mask). They rebuild the filter array and callsetsockopt(SOL_CAN_RAW, CAN_RAW_FILTER, ...).CANDeviceCollection::add_device()andremove_device()call them withget_recv_can_id()andget_recv_can_mask(). Until the first filter is added, and again after the last one is removed, the socket receives everything, so tools that register no devices are unaffected.CAN_RAW_ERR_FILTERis not touched.Filters are counted per (id, mask), because the arm or gripper collection and the master collection hold the same devices. A filter stays until every collection that added it has removed it, which is why
remove_recv_filter()takes the mask too. Re-adding a recv id to a collection adds the new device's filter before dropping the old one's, so frames for that id never stop in between.2. diagnose, health: also listen on 0x00. With only the devices' own filters, every frame that passes a filter matches a device, so the RID 7 hint could never fire. A motor whose master id was never set replies on 0x00, which neither tool registers. Both now add a filter for 0x00 themselves, so those replies still arrive and are counted as unmatched. That is the "passes a registered filter but matches no registered id" case from the issue. I put this in the tools rather than the library because the hint is theirs, but I can move it if you'd rather.
3. Drop the frames queued before the first receive filter. The socket receives everything until the first device is registered. Whatever arrives in that window is still queued after the filter goes in, and the first
recv_all()counted it as unmatched. On a busy bus that brings back the same false warning: 54 to 80 such frames in the flood test below. When the first filter goes in, the queue is now emptied first. Error frames in it still go intoBusStatus.Things to be aware of:
add_recv_filter()isn't in the Python bindings yet. I can add it here or separately.add_device()andremove_device(). I'll rebase whichever of the two lands second.From reading the code, the other commands keep working:
discover: I couldn't run it on vcan, because it reconfigures the bitrate. When it looks for a motor whose master id is unset, it already registers a device on 0x00, so those replies still get through.show_param,monitorand the status commands: they only read through registered devices, so the filters don't change what they show.CANSocket: they register nothing, so they still receive everything.Test
No motors needed. On
vcan0(GitHub Actions, Ubuntu 22.04 and 24.04), small scripts stand in for the motors and for another process on the bus.diagnose --explainandopenarm-can-healthrun againstmainand against this branch:maindiagnoselists 0x01 x117, 0x02 x117 and 0x7ff x234 as unmatched and prints the RID 7 warning, then "Nothing to report";healthprintsunmatched:lines for the same idsThe counts are from 24.04, and 22.04 gave the same picture. With the first commit alone, the 0x00 motor goes unreported, and 54 to 80 flood frames from before the filter still show up as unmatched. Commits 2 and 3 fix those two cases.
A small program also checks the socket API directly:
With the first commit alone, only the queued-frames case fails.
Run: https://github.com/pradeepsuryad/openarm_can/actions/runs/42085191330
The scripts are on a separate branch of my fork. I can add the socket check to the repository as a test if you'd like one.