Apply listen-only mode requested via ip link - #20
Open
knakazawa-ics wants to merge 1 commit into
Open
Conversation
The kernel module (intrepidcs/intrepid-socketcan-kernel-module#25) encodes CAN_CTRLMODE_LISTENONLY in a spare high bit of the settings-change report. Decode it and toggle CAN_SETTINGS.Mode between LISTEN_ONLY and NORMAL through IDeviceSettings::getMutableCANSettingsFor() when applying, alongside the existing baudrate handling. Other modes (LOOPBACK, LISTEN_ALL, DISABLE) that may have been configured with other tools are left untouched by bitrate-only changes, and the mode is only staged once the requested baudrates have been accepted, so a rejected baudrate cannot leave a half-staged mode behind. The device lookup now takes openDevicesMutex (this path previously scanned openDevices unlocked while the search thread could mutate it), and settings are staged and applied outside the lock so the search and RPC threads are not blocked while IDeviceSettings::apply() runs, which can take several seconds. Signed-off-by: Koki Nakazawa <knakazawa@intrepidcs.com>
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
1 participant
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.
Companion to intrepidcs/intrepid-socketcan-kernel-module#27; the two are meant to
land together.
The kernel module encodes
CAN_CTRLMODE_LISTENONLYin bit 30 of the settings-changereport. This PR masks the flag out of the bitrate and toggles
CAN_SETTINGS.Modebetween
LISTEN_ONLYandNORMALthroughIDeviceSettings::getMutableCANSettingsFor(), alongside the existing baudratehandling.
Design decisions (please push back if you disagree):
LISTEN_ALL, DISABLE) that may have been configured with other tools are left
untouched by bitrate-only changes.
cannot leave a half-staged mode in the settings buffer (which a later unrelated
apply() would otherwise flush to the device).
openDevicesMutex: this branch previously scannedopenDevicesunlocked while the search thread can erase entries (this is thebranch the PR extends, so the fix is included here rather than split out).
Settings are staged and applied outside the lock so the search and RPC threads
are not blocked while
IDeviceSettings::apply()runs, which can take severalseconds.
settings->apply()persists to EEPROM, same as the existing baudrate path. Thatmeans a listen-only setting survives daemon shutdown and is visible to other tools
(we hit exactly this during testing: a device left in listen-only kept withholding
ACKs on a bench bus afterwards, failing every other node's transmissions). If you
would rather use volatile settings (
apply(true)) for ip-link-driven changes, Ican change it, though that would also change the existing baudrate behavior.
version gate expects, so no version bump is included.
Tested end-to-end on Ubuntu 22.04.5 with a neoVI FIRE 3 and a ValueCAN 4-2, each
exercised as the SocketCAN device (details in the kernel PR), with
LIBICSNEO_USE_SERVD=0set for discovery (stock discovery is currently affected by #21). The daemon logs the decoded request: