feat: add software debounce filter for GPIO magnetic sensor + FTMS control point fixes#199
Conversation
The FTMS Control Point characteristic returned wrong result codes in several cases: - reset/startOrResume/stopOrPause without prior requestControl returned opCodeNotSupported (0x02) instead of controlNotPermitted (0x05) - stopOrPause with invalid parameter (not 1 or 2) returned opCodeNotSupported instead of invalidParameter (0x03) - stopOrPause with a truncated 1-byte buffer caused a RangeError crash (buffer underflow) that prevented indicate+callback from being called, hanging the BLE stack Also removes dead fallthrough code — all handled cases now return explicitly, so only the default case produces opCodeNotSupported. Adds 11 uvu tests covering all control point code paths.
Hardware bounce from magnetic flywheel sensors can send phantom ticks (~0.5ms apart) that corrupt stroke data. This middleware layer filters bounces by validating delta-times against a rolling median buffer. - Rejects deltas below 1ms (absolute minimum) - Rejects deltas below 40% of median (physically impossible acceleration) - Uses fixed-size circular buffer for performance (no allocations in hot path)
|
Am I misunderstanding something, or couldn't the noise be filtered out already, via config? In the docs, it's described as Switch Bounce, I think: |
Yes this feature is already available. Not the 40% deviation though, but I personally dont really support this feature (on ESPRM I have something similar that can be enabled for extreme cases but it is known to cause some times more problems that solves). The main message in terms of denounce is that try to avoid solving a hardware issue with a software solution. |
|
I'm extremely reluctant to get a rolling median filter back. Especially since we have a debounce filter in place in pigpio to fix this specific issue. 0.5ms is something that filter can handle easily (set it to 750 ns). Our design is based on accepting noise and treating datapoints as untrusted. The new cyclic error correction filter (0.9.7) also detects outliers and gives them a low weight, which works reasonably well on noisy machines. This proposed aporoach essentially sets our approach back three years (to version 0.8.1) where noise prediction and rejection was a thing. It never worked well on scale. I think our current approach is more robust. |
In 0.8.1 we used a similar approach. We used prediction to accept/reject datapoints. The big issue is that configuration really becomes a thing: you get weird abstract parameters like maximum change percentage in currentDt, which works well until someone faster comes along and he knows how to increase a flywheel speed better. We abandoned it as it indeed creates much more problems than it solves. A couple of noisy datapoints and everything is all over the place, often permanently.
This, and the pigpio debounce filter is there for a reason as well. Beyond that the new CECFilter will reduce its weight if it is outside the normal ranges (worked well with your Kayak) and the weighted Regression will mostly ignore it. My tests with CECFilter show that this type of noise will still create some disturbance, but it will largely be ignored. |
|
That makes total sense, especially regarding the 'someone faster' edge case. I did a bit of assuming, but I completely understand why a hard software filter is dangerous here and why the engine relies on probabilistically weighting untrusted data points instead. I will strip the GPIO debounce filter out of this branch entirely and leave only the FTMS control point fixes. I'll push the updated commit and adjust the PR title shortly (as I'm at school now). |
Great idea! |
|
okay all done, debounce filter removed. the PR only contains the FTMS fixes. should be all good for review |
There was a problem hiding this comment.
Please remove these changes. Deleting comments we need for maintenance is not a good way to go
There was a problem hiding this comment.
im super sorry about that! The comment removal was an unintended side effect when I stripped out the debounce filter changes, and I thought I added those back in. I've restored all the comments in GpioTimerService.js. The file should now match the base branch exactly with no changes from this PR.
Comments were unintentionally removed when stripping the debounce filter changes. File now matches the base branch with no modifications.
This PR includes two improvements: