fix(ble): FTMS control point - stopOrPause and setIndoorBikeSimulationParameters bug fixes#187
Open
cwklurks wants to merge 3 commits intolaberning:mainfrom
Open
fix(ble): FTMS control point - stopOrPause and setIndoorBikeSimulationParameters bug fixes#187cwklurks wants to merge 3 commits intolaberning:mainfrom
cwklurks wants to merge 3 commits intolaberning:mainfrom
Conversation
…nParameters - Fix stopOrPause with invalid parameter now returns invalidParameter response instead of hanging BLE stack - Fix setIndoorBikeSimulationParameters now requires requestControl like other commands
- stopOrPause with invalid param returns invalidParameter instead of silently dropping (prevents BLE stack hang) - setIndoorBikeSimulationParameters now requires requestControl first - add buffer length validation to prevent crashes on truncated packets - add 4 edge case tests for malformed packets and invalid params
fix(ble): FTMS control point bug fixes and buffer validation
|
Hi, this repo is dead, use this fork instead: https://github.com/JaapvanEkris/openrowingmonitor Its being actively developed and years ahead of this including full BLE protocol rewrite (including stack, profiles and communication layer). Thanks |
Author
|
Ah, I was wondering about that. I forked the wrong one. I'll see what changes I can help with on the updated repo. thank u! |
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
2 participants
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.
Hi! First-time contributor here, and I'm a high-school rower as well. I found two bugs in the FTMS control point characteristic while reading through the BLE implementation and wanted to fix them.
Bugs Fixed
1.
stopOrPausenever responds on invalid parameterWhen
stopOrPausereceives acontrolParameterother than1(stop) or2(pause), the error islogged but the BLE callback is never called. This leaves the connected app hanging indefinitely with no
response.
Fix: Return
invalidParameter(0x03) for values outside{1, 2}, and validate the parameter byteis present in the buffer.
2.
setIndoorBikeSimulationParametersbypassesrequestControlcheckAll other commands go through
handleSimpleCommand, which enforces thecontrolledstate.setIndoorBikeSimulationParametersskips this and invokes the callback directly, succeeding without aprior
requestControlcall. This violates the FTMS spec.Fix: New
handleSimParametersmethod that checkscontrolledstate and returnscontrolNotPermitted(0x05) when appropriate. Also adds buffer length validation (requires 7 bytes) toprevent out-of-bounds reads on truncated payloads.
Test Coverage
Added 23 tests for
FitnessMachineControlPointCharacteristiccovering:requestControl-> controlled state machineUses Node's
module.register()to mock@abandonware/blenowithout additional dependencies.Notes
npm test- all existing and new tests pass