fix(mcp2515): persist acceptance filter across power cycles

[slothish]

Problem

SetAcceptanceFilter() correctly writes filter registers and sets RXMODE, but
the 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:

1. Persist filter config: SetAcceptanceFilter() stores the config in
   m_filter_cfg / m_filter_set after successfully writing registers.

2. Re-apply on Start(): After init, if a filter was previously
   configured, SetAcceptanceFilter() is called again to restore RXMODE.
   A log error is emitted if the restore fails.

3. Add missing REG_RXB1CTRL define (0x70): was absent from
   mcp2515_regdef.h; SetAcceptanceFilter() was setting RXMODE only on
   RXB0 as a result.

Compatibility

Callers that never call SetAcceptanceFilter() are unaffected — m_filter_set
defaults to false, so Start() takes no new action. Callers that pass
all-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 the
chip out of CONFIG mode and called pcp::SetPowerMode(On). During the brief
window 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 (probably
a _locked variant of SetAcceptanceFilter callable from Start) is left
as a follow-up so this PR stays focused on the persistence bug.

[markwj]

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?

[slothish]

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?

[dexterbg]

The approach sounds generally right, but also depends on the application (vehicle) setting the filter before calling Start(), and SetAcceptanceFilter() probably needs to skip changing the mode if not yet started (?).

When implementing the filters, I thought about adding a Start() variant that additionally accepts a filter configuration as a parameter, so this becomes atomic on the application level. Maybe that's a sensible addition here.

[markwj]

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.

[slothish]

The "skip mode change when not yet started" branch is easy — SetAcceptanceFilter checks m_canctrl_mode (or m_mode == CAN_MODE_OFF) and skips the enter/exit config bracket. Fine for the existing runtime-reconfig case too.

The atomic Start(mode, speed, filter) is the better primitive though — no ordering contract for the caller, and post-Start reconfigs still go through SetAcceptanceFilter as today. Rough sketch:

• New Start overload (in canbus base, I think — see below)
• mcp2515 implementation writes filter regs while still in config mode, before the active/listen ChangeMode
• RegisterCanBus gets a variant that forwards the filter
• SetAcceptanceFilter gets the not-started branch for symmetry

One open question on the API shape: both mcp2515 and esp32can have hardware filters today, but with their own config structs (mcp2515_filter_config_t vs esp32can_filter_config_t). For the atomic Start to be useful at the application level it probably wants a generic filter type in canbus base — otherwise the vehicle still has to know which controller it's talking to, which mostly defeats the point. Bigger change up front, but it sets the shape correctly for both controllers.

Which way do you want to take it?

[dexterbg]

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.

[slothish]

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.

[dexterbg]

Andreas, what's the outcome of your tests? PR #1404 depends on this, and also tagged as draft. What about the later PRs, can they be merged without these two?

[slothish]

Short version: the ISR-storm premise behind this PR didn't survive testing, but I haven't run the test that settles it.

Unfiltered FCAN at ~1700 fps (about 2x the OBC charge rate that triggered the original reboot loop) ran 123 s with no TWDT. A longer unfiltered CCS run, ~390 s, did reboot, but the boot log said PowerOn (9/14) with crash counters at 0. That's not a watchdog reset, that's the 12V rail dropping out. My harness is AWG 24 and I'm rewiring it.

So the reboot loop I blamed on the ISR storm looks like a supply problem on my car, not frame throughput. Still missing is a full multi-hour unfiltered AC charge, longest unfiltered run so far is about 6.5 minutes. I'll run that once the harness is redone.

Chasing the same "module goes deaf" symptom did turn up a real driver bug: INT GPIO was edge-triggered and xQueueSendFromISR was unchecked, so a dropped notification left the line low with no further edge and the RX path wedged for good. That's #1461 and #1462, verified on the car over ~7M frames with zero wedge resets.

On merging: those two stand alone. They touch the ISR and interrupt setup, not Start() or SetAcceptanceFilter(), so no dependency on this PR or #1404, and both show mergeable against master. #1462 includes #1461's commit so it wants #1461 in first. #1430 is independent as well, it doesn't touch mcp2515 or the filter and merges clean on current master. If it lands before #1404 I'll rebase #1404 on top.

Suggest taking #1461 and #1462 now, leaving this and #1404 in draft until I have the charge test. If it comes back clean I'll close both and drop the filter from my module.