fix: correct FTMS Control Point error codes per Bluetooth spec#189
fix: correct FTMS Control Point error codes per Bluetooth spec#189cwklurks wants to merge 2 commits intoJaapvanEkris:mainfrom
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.
Abasz
left a comment
Abasz
left a comment
There was a problem hiding this comment.
Thanks for your contributions. Can you please test FTMS with EXR and ErgZone ?
Technically your changes should not make a different but the connection and communication flow sometimes not perfect on the other side and they apply logic that may break things (like sending reset randomly which ORM for instance cant or don not want to handle etc.)
Also the nested if else statement flow is getting quite difficult to understand (this is not your code its the way it was implemented originally :)) and I wonder whether you are willing to update things to use guard clauses instead (like in the case ControlPointOpCode.stopOrPause: { case where we could simply use this.#controlled for the guard)
Thanks!
| return this.#buildResponse(opCode, ResultOpCode.success) | ||
| } else { | ||
| log.error('FTMS: startOrResume attempted before RequestControl') | ||
| return this.#buildResponse(opCode, ResultOpCode.controlNotPermitted) |
There was a problem hiding this comment.
This needs to be specifically tested as ErgZone had issues with the start and resume flow.
There was a problem hiding this comment.
This needs to be specifically tested as ErgZone had issues with the start and resume flow.
I can test with EXR and ErgZone in the weekend.
|
Thank you! I refactored to use guard clauses across all three cases (reset, startOrResume, stopOrPause). Also flattened the nested if/else if inside stopOrPause into sequential guards while I was at it. |
3 participants
Fixes three issues in
FitnessMachineControlPointCharacteristic:reset,startOrResume, andstopOrPausewithout priorrequestControlreturnedopCodeNotSupported(0x02) — should becontrolNotPermitted(0x05) per FTMS specstopOrPausewith an invalid control parameter returnedopCodeNotSupported— should beinvalidParameter(0x03)stopOrPausewith a truncated 1-byte buffer threwRangeError, preventingindicate()/callback()from firing and hanging the BLE stackAlso cleans up dead fallthrough code — all handled cases now return explicitly so only
defaultproducesopCodeNotSupported.Adds 11 uvu tests for the control point characteristic.