Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ANT command status response (page 71) #222

Merged
merged 2 commits into from
Feb 3, 2021

Conversation

switchabl
Copy link
Contributor

The handling of page 71 command status pages is broken. The data bytes need to be the raw bytes from the control command, but we are trying to pass floats instead, causing FortiusANT to crash (#101 (comment)). Also the max value for the sequence # should be 254, as 255 indicates no command received [0]

Awaiting confirmation from @smithnb that this indeed fixes the issue.

[0] Section 8.12.3 of ANT+ FE specification

@WouterJD
Copy link
Owner

Some questions:

1

info[8] is unpaged by msgUnpage48_BasicResistance()

p71_Data4 = info[8]
could be replaced by
p71_Data4 = Grade

2

Please add comment # rollover at 254, as 255 indicates no command received
Should the intial p71_SequenceNr = 0 be p71_SequenceNr = 255 for the same reason?

3

p71_Data3 =  int(TacxTrainer.TargetPower) & 0x00ff
p71_Data4 = (int(TacxTrainer.TargetPower) & 0xff00) >> 8

is replaced by

p71_Data3                   = info[7]
p71_Data4                   = info[8]

although it should be the same
TacxTrainer.SetPower(ant.msgUnpage49_TargetPower(info))
could better be split into

TargetPower = ant.msgUnpage49_TargetPower(info))
TacxTrainer.SetPower(TargetPower)

and then TargetPower be used, in analogy with Grade above

4

Like previous; I think the result is the same:

p71_Data2                   = WindResistance
p71_Data3                   = WindSpeed
p71_Data4                   = DraftingFactor

Or do I miss the essential for 3 and 4?

@switchabl
Copy link
Contributor Author

1, 3, 4: The unpage functions do unit conversions too. So TargetPower is actually info[7-8] / 4. In that case it just means that we are sending the wrong value back. And e.g. WindResistance is

    WindResistance = tuple[nWindResistanceCoefficient]
    if WindResistance == 0xff:
        WindResistance = 0.51
    else:
        WindResistance = WindResistance * 0.01 # kg/m

So we are trying to pass a float (which is what causes the crash). I don't particularly like accessing info directly but I don't see a good alternative here (short of moving the unit conversion code out of unpage). Trying to reverse the calculation will not work in all cases and it is also really messy. In any case it ensures that we are really sending back the same raw bytes that we received (which is what is required). So I could maybe just add comments saying that 1) the raw bytes are expected by the CTP and 2) what they contain?

2: Yes, good catch. Also I will add a comment.

@WouterJD
Copy link
Owner

WouterJD commented Jan 27, 2021

@switchabl agree; plse check for consistency👍

@switchabl
Copy link
Contributor Author

Hopefully everything is consistent and comments are clear now. Let me know if there is anything else.

@WouterJD WouterJD merged commit 058703b into WouterJD:master Feb 3, 2021
@WouterJD
Copy link
Owner

WouterJD commented Feb 3, 2021

Thanks and done

WouterJD added a commit that referenced this pull request Feb 4, 2021
WouterJD added a commit that referenced this pull request Feb 4, 2021
WouterJD added a commit that referenced this pull request Feb 4, 2021
* #216 GUI crash raspberry v1

* #216 GUI crash raspberry v2

* #216 GUI crash raspberry v2

* GUI improvements, Sponsor, Calibrate

* #189 preparation

* #189 preparation

* Update README.md

* Update README.md

* Fix ANT command status response (page 71) (#222)

* Fix ANT command status response (page 71)

* Fix initial page 71 sequence number, add comments explaining page 71 data

* #216 GUI crash raspberry v4

* Fix ANT command status response (page 71) #222

Change history updated

* #216 GUI crash raspberry v5

* #216 GUI crash raspberry v1

* #216 GUI crash raspberry v2

* #216 GUI crash raspberry v2

* GUI improvements, Sponsor, Calibrate and #222

* #216 GUI crash raspberry v4

* #216 GUI crash raspberry v5

Co-authored-by: WouterJD <[email protected]>
Co-authored-by: Wouter Dubbeldam <[email protected]>
Co-authored-by: switchabl <[email protected]>
@switchabl switchabl deleted the fix_command_status branch February 10, 2021 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants