fix(mcp2515): persist acceptance filter across power cycles#1403
fix(mcp2515): persist acceptance filter across power cycles#1403slothish wants to merge 1 commit into
Conversation
SetAcceptanceFilter() stores the filter config; Start() re-applies it after init so SetPowerMode(On) / ignition cycles no longer silently revert RXMODE to 3 (receive-all), bypassing the hardware filter. Also adds REG_RXB1CTRL define (was missing) and switches RXB0/RXB1 to RXMODE=0 (filter-matched) after writing filter registers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
If this is a race condition between Start and SetAcceptanceFilter, surely a better solution would be to either set the filter before Start, or during the Start call? Or am I missing something? |
|
I was reluctant to touch the Start sequence more than necessary in this PR, so I parked it as a known limitation. But you're right that it's the cleaner fix. What I had in mind was to split SetAcceptanceFilter into the public entry (lock + config-mode in/out + register writes) and a private ApplyFilterRegs(cfg) helper that only does the register writes. Start() would call the helper directly while it's still in config mode (after the CNFx writes, before ChangeMode(NORMAL/LISTEN)), and the trailing re-apply block in Start can go away. Does that approach sound right to you? |
|
The approach sounds generally right, but also depends on the application (vehicle) setting the filter before calling When implementing the filters, I thought about adding a |
|
It seems sensible to me to (a) allow the filters to be defined before the Start, and/or (b) introduce a Start variant with the filters passed as parameters. Either would avoid the race condition. |
|
The "skip mode change when not yet started" branch is easy — The atomic
One open question on the API shape: both Which way do you want to take it? |
|
There is no sensible way to remove the controller dependency regarding hardware filters. The acceptance filter capabilities and bit layouts are hardware specific, and need to be defined with the CAN controller in mind, as the capabilities and restrictions to cope with are very different. There of course is the option to create a filter compiler that creates a working hw configuration from a software/generic filter configuration, but this will never be able to build an exact mapping for both controller types, so the application still needs to be written with the edge cases in mind the actual hardware cannot fulfill. Also that compiler would be quite complex, and hardware filters haven't really been needed for any application up to now, probably only ever will be for the SPI/MCP controlled buses. Software filters normally are sufficient, much easier to define and maintain. IOW the actual need doesn't justify the effort. You can create a virtual base class or union type covering both filter specifications, so you can define generic API signatures, with the actual bus implementation then picking the filter spec it needs. But it would also be OK to put this on the list for the next release. |
|
This is starting to feel like a sidequest. Since I'm the only vehicle module currently relying on mcp2515 hardware filters, I'd rather try to remove the need for them before digging this hole any deeper. I'll run a few charging sessions on my car without the filter active to see if I can avoid the ISR storm — if so, the filter API question becomes moot. |
Problem
SetAcceptanceFilter()correctly writes filter registers and sets RXMODE, butthe configuration is lost on every power cycle.
Start()hardcodes RXMODE=3(receive-all) in RXB0CTRL/RXB1CTRL, silently bypassing any previously
configured hardware filter after
SetPowerMode(On)or ignition cycles.This means vehicles that configure an acceptance filter (e.g. to drop
high-frequency frames that would otherwise saturate the ISR and starve the
Events task) get no filtering in practice — the filter only holds until the
next power-mode transition.
Observed symptom on VW e-Golf / J533 harness: CAN2 (FCAN) runs ~860 fps
during OBC charging. Without a working filter, all frames reach the ISR →
Events task starved → TWDT reset loop → module reboots continuously during
charging.
Fix
Three changes:
Persist filter config:
SetAcceptanceFilter()stores the config inm_filter_cfg/m_filter_setafter successfully writing registers.Re-apply on
Start(): After init, if a filter was previouslyconfigured,
SetAcceptanceFilter()is called again to restore RXMODE.A log error is emitted if the restore fails.
Add missing
REG_RXB1CTRLdefine (0x70): was absent frommcp2515_regdef.h;SetAcceptanceFilter()was setting RXMODE only onRXB0 as a result.
Compatibility
Callers that never call
SetAcceptanceFilter()are unaffected —m_filter_setdefaults to false, so
Start()takes no new action. Callers that passall-zero masks continue to receive all frames (RXMODE=0 with an all-pass
mask is equivalent to RXMODE=3).
NIU GTevo has
SetAcceptanceFilter()commented out — no impact.Known limitation (follow-up)
The filter is re-applied after
Start()has already transitioned thechip out of CONFIG mode and called
pcp::SetPowerMode(On). During the briefwindow between mode-change and re-apply (typically <20 ms), the chip is in
NORMAL/LISTEN mode with RXMODE=3, so unfiltered frames will reach the ISR.
On a hot bus this can still produce a short ISR burst on every restart.
A cleaner fix would write the filter registers while still in CONFIG mode
during
Start(), eliminating the window entirely. That refactor (probablya
_lockedvariant ofSetAcceptanceFiltercallable fromStart) is leftas a follow-up so this PR stays focused on the persistence bug.