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

Issues in broadcasting powerdata 2 Rouvy (adjust ModelNumber_FE) #184

Closed
cyclingflow opened this issue Dec 26, 2020 · 40 comments
Closed

Issues in broadcasting powerdata 2 Rouvy (adjust ModelNumber_FE) #184

cyclingflow opened this issue Dec 26, 2020 · 40 comments
Assignees
Labels
bug Something isn't working implemented

Comments

@cyclingflow
Copy link

Over the last weaks i noticed regular dips in power display at Rouvy without such dips in ForitusAnt. Moreover, less frequent i also noted high power spikes (say 4000-5000 watt).

I initially studied the logs and json logs but could not find evidence of an error. At the same time , i was working at receiving and logging my external bike power meter in the json log, and using accumulated power.

Suddenly i realized, this might be connected to the reason for the issues i experienced: the way accumulated power is calculated in AntPWR.py

(issues and solution below)

@cyclingflow
Copy link
Author

cyclingflow commented Dec 26, 2020

When studying the way my stages power meter does it, i got the following impression:

  1. Stages does not reset or reinitialize the eventcounter or accumulated power. What it does can be conceived as
    internally increase the event counter en accumulatedpower at the required precision, and then only broadcast the last 8 / 16 bits.
    Eventcounter and accumulated power can be updated using:
    EventCount = (EventCount+1)%256 AccumulatedPower = (AccumulatedPower+CurrentPower)%65536
    Conversely, the average power can be computed at all times by:
    AvePower = ((_AccumulatedPower - PreviousAccumulatedPower)%65536) / ((_EventCount - PreviousEventcount)%256)

  2. When is eventcount/accumulated power updated?
    Currently, antPWR,py does not update the accumulated power when it broadcasts factory messages, while it still does increase the event count.
    I currently believe that accumulated power and event count should be always updated, when broadcasting at regular intervals.
    Moreover, Stages only updates the eventcount when current power >0 and increases the event counter one more time if you stop and current power becomes zero.
    So we get:
    if LastPower>0 or CurrentPower>0: EventCount = (EventCount+1)%256 AccumulatedPower = (AccumulatedPower+CurrentPower)%65536 LastPower = CurrentPower
    This works fine for me. Today i completed a 40km ride without any dips or spikes.
    @WouterJD : you can find my updated antPWR.py in my fork if you are interested

(Sorry for formatting errors, dont know exactly why this happen, working remote now)

@WouterJD
Copy link
Owner

@cyclingflow I will study the issue shortly
Thanks for analysis👍

@WouterJD
Copy link
Owner

Referring to D00001086_ANT+Device_Profile-_Bicycle_Power_Rev_5.1.pdf

8.1 Update Event Count
The update event count field is incremented each time the information in the message is updated.
...
The Power-only page... must be sent each time the update event counter is incremented.

I agree that the current implementation is incorrect, since EventCounter is incremented also for the interleaved messages (and AccumulatedPower is not).

I think there should be a separate counter to decide for interleaving, independant of the EventCounter which is incremented only when the AccumulatedPower is calculated AND msgPage16_PowerOnly is sent.


Note that the same construct exists in FE-C.

@WouterJD
Copy link
Owner

Question: why do you not broadcast power when power=0?

WouterJD added a commit that referenced this issue Dec 27, 2020
@WouterJD
Copy link
Owner

WouterJD commented Dec 27, 2020

I have reviewed antCTRL.py, antFE.py, antHRM.py, antPWR.py and antCSC.py

  • antFE and andPWR had the same issue
  • rollover was not implemented according definition but a reset was done
  • the modules are in sync

I will test myself tomorrow, perhaps you want to take a look as well.

Branch: 4.2-Quality-upgrade

@WouterJD WouterJD added bug Something isn't working under investigation Being studied for implementation in next version labels Dec 27, 2020
@cyclingflow
Copy link
Author

Question: why do you not broadcast power when power=0?

To be precise: I should be always broadcasting, just not updating the event counter and accumulated power.
Otherwise i made an error.

I did this because it clearly is the way my stages power meter is performing right now. I did not have access to the official specs, it required a registration with manual review.

My perception of the mechanism:

  1. it is designed to have accurate transmissions of values even in the event of transmission/reception problems, including frequent dropouts, while minimizing the amount of bits transferred
  2. by not incrementing the event counter while power is zero, you effectively transmit the actual cumulative time the cyclist was pedaling - even in the event of dropouts, while the receiving end can easily keep track itself of the total duration of the exercise.
    I now also note that it conforms to the standard: event counter has to be updated when information is updated.

@cyclingflow
Copy link
Author

cyclingflow commented Dec 27, 2020

I have reviewed antCTRL.py, antFE.py, antHRM.py, antPWR.py and antCSC.py

Good you did, i did not get around to it yet. I'll take a look and update my version,
such that it will also test those.

I think there should be a separate counter to decide for interleaving, independant of the EventCounter which is incremented only when the AccumulatedPower is calculated AND msgPage16_PowerOnly is sent.

At first i did not understand what you mean, but now i do, and I agree. Would be nicer, including the fact that it means that the receiving end will receive 'factory information' after connection (after a short period of time), even if you do not immediately start pedaling.

As a side-note: i don't think many power meters worry about this, because they stop broadcasting anyway when you stop pedaling, to save battery power.

@cyclingflow
Copy link
Author

cyclingflow commented Dec 27, 2020

w.r.t. Rouvy: the power dips have always been there (and we know why now), but the power spikes started around 15 nov. Possibly related to an update of Rouvy AR only. I'm not sure Rouvy Workouts has it, because i do not see it during workouts.
(This morning i reviewed all activities to see when the abnormal max power ratings started, in the range of 4000-7000W.)

It is a bit difficult to keep track off, because i have the impression that when you start Rouvy AR through Rouvy Workouts, it uses the workouts connection, and if you start Rouvy AR directly, it uses a little bit different connection interface.

Anyway, it seems to be gone now, with the update of the 'reset' mechanism.

@WouterJD WouterJD self-assigned this Dec 27, 2020
@cyclingflow
Copy link
Author

I just took a look at the current updated antPWR.py
@WouterJD
Question for you: why whould you not be updating cumulative time and power when interleaving?

I was under the impression that broadcast was called every quarter second, whether interleaving or not, and therefore you are effectively 'polling' power every quarter second.
So, i felt you should update cumulative time (counter) and power once every 'poll', whether you broadcast it or not.

@WouterJD
Copy link
Owner

Question for you: why whould you not be updating cumulative time and power when interleaving?

That's why I copied the ANTdefinition in the comment: for each increment of the EventCount the message must be broadcasted.

I must say, the interleaving was always a bot misterious for me; that's why I reread the ANT profile definitions today. I think it should be done as coded now - I will test tomorrow.

@WouterJD
Copy link
Owner

I was under the impression that broadcast was called every quarter second

That is correct. The power-message is only sent when there is NO interleave message. Only one message is sent per call.

@WouterJD
Copy link
Owner

This is compliant with antHRM. All I did there is rename the variable into Interleave for consistency

@WouterJD
Copy link
Owner

Of course interested in further advise regarding implementation of the ANTdefinition

@cyclingflow
Copy link
Author

cyclingflow commented Dec 27, 2020

This is compliant with antHRM.

Thx for the explanation. I understand now. So, it seems to me, the standard either does something slightly irregular to cumulative power (and may be one should adjust when receiving), or we might be required to broadcast interleave messages at other times.
(=EDIT, that does not make sense, reading about the ANT protocol)

I noticed that the event counter of my stages power meter also sometimes increases by more than one, so it did not worry me:
radio interference could also mean skipping some events.
I'll take a closer look at the stages power meter and intercept the interleave messages as well.

@cyclingflow
Copy link
Author

N.B. @WouterJD : i got type errors in antFE and antPWR w.r.t. the &= 0xff statements. Easy to correct.

@WouterJD
Copy link
Owner

I assume because they're not int?🤔

WouterJD added a commit that referenced this issue Dec 27, 2020
WouterJD added a commit that referenced this issue Dec 27, 2020
WouterJD added a commit that referenced this issue Dec 27, 2020
@cyclingflow
Copy link
Author

cyclingflow commented Dec 28, 2020

tricky business, understanding these protocols.
Unfortunately, i introduced two errors and caught only one in my Json log code. So i have to do some more testing

@cyclingflow
Copy link
Author

For now i can say:

  1. if my power meter is supposed to broadcast (page 16) at every increment of the event counter,
    i'm not catching all page16 broadcasted. Frequently, the eventcounter is increased by 2 or 3.
  2. Once and a while, i receive two pages at one quarter second (16 and 18, 18 and 18, or 16 and 19)
    (18 is the crank torque page)

@WouterJD
Copy link
Owner

I will do a test this evening, repeating the same ride as before to see whether something changed.
I had some weard changes as well, but could not explain.
I will create the json file and check it whether I see strange behaviour.

@switchabl
Copy link
Contributor

2. Once and a while, i receive two pages at one quarter second (16 and 18, 18 and 18, or 16 and 19)
   (18 is the crank torque page)

The channel period is 8182 (rather than 8192) counts, that is actually slightly more than 4Hz. Many power meters can also be switched to 8Hz mode (e.g. my Edge 520 automatically does that when it connects to the Vector pedals). But a receiver set to 4Hz shouldn't see those.

@WouterJD
Copy link
Owner

I very much prefer to implement according specification, unless proven it must be done otherwise

Instead of reverse engineering a specific ANT-implementation (who says it's implement ed correctly)

@switchabl
Copy link
Contributor

Yes, agreed. I was just explaining why you would see more than 1 page per quarter second occasionally. This should be the same for any compliant device (8182 period is in the specification). It also means you will have an occasional tx fail (missing data) when you try to send a new one exactly every 0.25s. But messages get lost sometimes anyway, so it should not cause any real problems.

It is still interesting to see what others have done (also how the ANT+ simulator does it). Because sometimes the specification is not quite clear or we might have missed something.

@cyclingflow
Copy link
Author

cyclingflow commented Dec 28, 2020

It is still interesting to see what others have done (also how the ANT+ simulator does it). Because sometimes the specification is not quite clear or we might have missed something.

That would be my point. I'm not convinced i catch all pages i should from my bike meter, and wonder we send enough.
I have also noted differences in average power from the same power meter, between Bike computer fit files, FortiusAnt and Rouvy, beyond what you would expect given all the other factors.

The FE spec includes a duration (accumulated time), the power/cadence not. I might have missed it.
For power meters, how does the spec define calculation of total Joules registered by the power meter,
given that we skip a few page16 every 32 packets.

@cyclingflow
Copy link
Author

cyclingflow commented Dec 28, 2020

Instead of reverse engineering a specific ANT-implementation (who says it's implement ed correctly)

I would not dare to say so. But they do have ANT certification, FortiusAnt does not. :-)

But messages get lost sometimes anyway, so it should not cause any real problems.

Yep, and the experience of the users is, FortiusAnt 'works' with several VR software variations. So it's just about a few details or quirks.

@WouterJD
Copy link
Owner

I absolutely agree on what is said; existing certified devices can teach how it should be - perhaps to properly understand the spec.
After that, let's follow the spec.

And while cycling I realized there is another improvement to be made (in antPWR.py and antFE.py:
AccumulatedPower += CurrentPower
must be:
AccumulatedPower += max(0, CurrentPower)

because
D000001231_-_ANT+Device_Profile-Fitness_Equipment-Rev_5.0(6).pdf
9.1.1 Calculating accumulated values
NOTE: All accumulating message fields must use only positive values.

@WouterJD
Copy link
Owner

I did a test-ride today, same ride as some days ago and it felt a lot better.
After the "test" I realized I also reinstalled my laptop (windows 7->10 finally) and now use Rouvy AR. So many changes, but the result is good.

Let's proceed in improving the profiles.

@cyclingflow
Copy link
Author

NOTE: All accumulating message fields must use only positive values.

good point

@WouterJD
Copy link
Owner

WouterJD commented Dec 28, 2020

FortiusANT.2020-12-28 19-11-58.txt

I have looked at the Rouvy-ride from today and it shows some special occurences:

  • Grade = -4,685 is followed by -7,27
  • Grade = -7,27 is followed by -3,845
  • Grade = -1,445 is followed by 1,215

Maximum jumps experienced:

  • Grade = -2,795 is followed by -7,64 (-4,845)
  • Grade = -3,295 is followed by 1,37 (4,665)

Of course this gives sudden changes in requested power; although I did not experience it as too disturbing.

I'm curious how you guys look at this.

@cyclingflow
Copy link
Author

Awkward. What route? Some are much worse than the official ones.

However, most changes of those changes I wont feel. Anything below zero gives very slight changes, and I have adjusted it such
that beyond -5% nothing changes. Let's see what others say.

@cyclingflow
Copy link
Author

As a side note: i have experienced that changing the type of FE-C you send to Rouvy AR, changes the scaling
of the grades you receive. (going from Neo to Flux (s) or Gen 2.) But only/primarily at the positive side of grades.

@switchabl
Copy link
Contributor

As a side note: i have experienced that changing the type of FE-C you send to Rouvy AR, changes the scaling
of the grades you receive. (going from Neo to Flux (s) or Gen 2.) But only/primarily at the positive side of grades.

That is interesting. We could send different trainer types depending on the brake connected so people get some automatic scaling without the "-G" option. E.g. maybe "Flow Smart" if we detect a magnetic brake. We would need some more data on the scaling function for different IDs.

@WouterJD
Copy link
Owner

@switchabl you refer to
ModelNumber_FE = 2875 # short antifier-value=0x8385, Tacx Neo=2875
in antDongle.py

Who can provide a list of modelnumbers?
I could simply run a test Rouvy against FortiusAnt -s to see whether there are differences.

@cyclingflow
Copy link
Author

cyclingflow commented Dec 30, 2020

ModelNumber_FE = 2900 # Tacx Flux (S) Smart= 2900
ModelNumber_FE = 2980 # Tacx Flux = 2980 Gen 2

what i have used so far

trial and error, based upon the tacx numbers :-)

@switchabl
Copy link
Contributor

switchabl commented Dec 30, 2020

I somehow thought @totalreverse had a list, but apparently not. Tacx Flow Smart would be particularly interesting. If they have implemented that, that could work well for all magnetic brakes.

My guess would be ModelNumber_FE = 2240, but I have no way to check.

@cyclingflow
Copy link
Author

cyclingflow commented Dec 30, 2020

i was just checking. it works
ModelNumber_FE = 2240 #Tacx Flow Smart
ModelNumber_FE = 2180 #Tacx Vortex Smart
ModelNumber_FE = 2780 # Tacx Bushido Smart

does not work
ModelNumber_FE = 2400 # satori
makes sense, because resistance is not controllable by ant i believe

@cyclingflow
Copy link
Author

cyclingflow commented Dec 30, 2020

I just also have taken a look at the grade re-scaling: no difference in Rouvy AR between 2240 Flow Smart or 2900 Flux Smart:
negative grades are not changed, positive grades are non-linearly re-scaled to a max grade of 12.7% (12.68 i believe)

So perhaps choosing between NEO for motor brakes and Flux for magnetic brakes -depending upon the brake type connected - would be sufficient for most users in Rouvy AR.

I'll do a full ride with flow smart selected in AR to see if anything else seems to be affected.

@WouterJD WouterJD changed the title Issues in broadcasting powerdata 2 Rouvy and solution Issues in broadcasting powerdata 2 Rouvy (adjust ModelNumber_FE) Dec 31, 2020
@Sat2Freak
Copy link

Hi Wouter,

Any idea when the incorrect power values, which appear in Rouvy, will be solved?
I thought this would be the case in version 5.0, but it appears to be the same as in previous versions (some peaks with values around 5000 Watt, and some drops...

Thanks for keeping up the good work!

@WouterJD
Copy link
Owner

WouterJD commented Jan 9, 2021

Please read the latest messages in #173, and test version 5.1 (see #198).
There's a fair chance it will resolve your issue

@WouterJD WouterJD added implemented and removed under investigation Being studied for implementation in next version labels Jan 9, 2021
@Sat2Freak
Copy link

Thanks Wouter. The power issues in Rouvy are gone with the test version! Once again : nice work!

@WouterJD
Copy link
Owner

WouterJD commented Jan 9, 2021

Thanks for confirmation; I will close issue.
If there'se anyting else, please raise a new issue

@WouterJD WouterJD closed this as completed Jan 9, 2021
WouterJD added a commit that referenced this issue Jan 21, 2021
* #184 Power in Rouvy issue

* #173 Version 4.0 Communicates Much Higher Power vs. 3.8

* #184 Power in Rouvy issue #2

* #184 Power in Rouvy issue #2

* #184 Power in Rouvy issue #3

* Fortius Antifier v4.2 test IV

* Fortius Antifier v5.1 test I

* Fortius Antifier v5.1 test II

* Fortius Antifier v5.1 test IIb

* #201

* #201

* Fortius Antifier v5.1 test III

* heart -> Heart2

* heart -> Heart2

* heart* -> heart.jpg

* Fortius Antifier v5.1 test IV

* Fortius Antifier v5.1 test V

* Fortius Antifier v5.1 test VI

* Fortius Antifier v5.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working implemented
Projects
None yet
Development

No branches or pull requests

4 participants