tijmenvangulik / ErgometerJS

Java script ergometer driver for concept2 performance monitor with BLE/USB .
Other
112 stars 35 forks source link

Read the correct amount of bits for strokeCount #10

Closed AuspeXeu closed 4 years ago

tijmenvangulik commented 4 years ago

Thank you for the change. Changing it to an 8 bit value means that you can only have 256 strokes. That would be a bit short race. The manuals states: Stroke Count Lo, CSAFE_PM_GET_STROKESTATS Stroke Count Hi, So it is an 16 bits value and not 8. This fix can not be right. What exactly is the problem?

AuspeXeu commented 4 years ago

With a PM5 and the latest firmware, I get strokeCount = 256 after the first stroke and strokeCount = 512 after the second stroke. Which is obviously binary 1 and 10 so there seems to be something wrong with the layout of your data structure. With the above change, it correctly shows strokeCount = 1 and strokeCount = 2, respectively.

tijmenvangulik commented 4 years ago

This may be a case of that the low and the high bytes are swapped. I have seen that with other values. Some 16 bits values are read reverse..

Tijmen

On 13 Apr 2020, at 16:55, Chris Vaas notifications@github.com wrote:

With a PM5 and the latest firmware, I get strokeCount = 256 after the first stroke and strokeCount = 512 after the second stroke. Which is obviously binary 1 and 10 so there seems to be something wrong with the layout of your data structure. With the above change, it correctly shows strokeCount = 1 and strokeCount = 2, respectively.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/pull/10#issuecomment-612933299, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NWKXAXLN7I5VHU4VILRMMRV3ANCNFSM4MG7UHOA.

AuspeXeu commented 4 years ago

How did you solve this problem since changing the endianness does not work here, can you point me to a value where you already had this issue?

tijmenvangulik commented 4 years ago

You can test 2 combinations:

data.getUint8(ble.PM_Extra_Stroke_Data_BLE_Payload.STROKE_COUNT_HI)+data.getUint8(ble.PM_Extra_Stroke_Data_BLE_Payload.STROKE_COUNT_LO)*256

data.getUint8(ble.PM_Extra_Stroke_Data_BLE_Payload.STROKE_COUNT_LO)+data.getUint8(ble.PM_Extra_Stroke_Data_BLE_Payload.STROKE_COUNT_HI)*256

Thank you for helping making the api better.

Tijmen

On 13 Apr 2020, at 16:23, Chris Vaas notifications@github.com wrote:

You can view, comment on, or merge this pull request online at:

https://github.com/tijmenvangulik/ErgometerJS/pull/10

Commit Summary

• Read the correct amount of bits for strokeCount File Changes

• M api/typescript/ergometer/performancemonitorBle.ts (6) Patch Links:

https://github.com/tijmenvangulik/ErgometerJS/pull/10.patchhttps://github.com/tijmenvangulik/ErgometerJS/pull/10.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

AuspeXeu commented 4 years ago

Done. Works just fine now :)

tijmenvangulik commented 4 years ago

Great work!. If push your change , I will merge it.

Tijmen

On 13 Apr 2020, at 17:30, Chris Vaas notifications@github.com wrote:

Done. Works just fine now :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tijmenvangulik/ErgometerJS/pull/10#issuecomment-612949368, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVG7NVBRORFSL6ETPE4X4LRMMV2RANCNFSM4MG7UHOA.

AuspeXeu commented 4 years ago

It's already up-to-date.