Added tests for bike power functions #2174
Conversation
drmason789
force-pushed
the
bike_power_tests
branch
from
b81bab2
to
678facc
Compare
|
@cagnulein is this something you want to continue with?
|
|
…the power table
|
@drmason789 Actually I was looking to finalize the dynamic erg table for the bike where I don't have a static power table first. Do you agree? Of course the dynamic power table will be applied only to the bike with automatic resistance and without the static power table. |
|
@drmason789 i mean this one #2175 |
Yes. |
Definately. I took a quick look and will have some comments about this later. |
…esistance for bikes. Move some power-related functionality up the class heirarchy
- Disabled test for going below minimum cadence as there's no public way of setting a negative cadence. - Boundary tests mostly if not all passing. - Tests that resistanceFromPowerRequest inverts powerFromResistanceRequest still failing
- adjusted power conversion test to force no watt gain - adjusted bike::resistanceFromPowerRequest to make it easier to see what it's comparing and to cache calculations - many power conversion tests now passing, but some strange values remain in the failing tests
| } else if (requestResistance < min_resistance) { | ||
| requestResistance = min_resistance; | ||
| if(!this->resistanceLimits().contains(requestResistance)) { | ||
| requestResistance = resistanceLimits().clip(requestResistance); | ||
| } else if (requestResistance == 0) { | ||
| requestResistance = 1; | ||
| } |
There was a problem hiding this comment.
@cagnulein What's going on with these few bikes that have this form of clipping of the resistance request?
i.e. here the min_resistance constant is -20 and the max_resistance constant is 100, and you clip the requestedResistance to that range, except if it's 0 then you make it 1.
There was a problem hiding this comment.
This is also in the proformwifi and proformtelnet bikes.
There was a problem hiding this comment.
min_resistance can't never be less than 0 in QZ. I mean I never encountered a bike with a negative resistance. So the current master can't handle this. That's why for inclination, instead, i'm using a -100 value as a "no action" value (because inclination can go lower than 0 (but not to -100 of course)).
did I answer to your question?
There was a problem hiding this comment.
I'm aware of the use of -1 and -100 to specify "no action" for resistance and inclination. But I don't know the purpose of the -20..100 range you're using here.
I'll use 1..100 for this exercise unless you specify something else.
There was a problem hiding this comment.
BTW, you might have noticed in some test code I've switched to C++17 and used the std::optional template. This enables you to specify something like
std::optional<resistance_t> resistanceRequest;
and use resistanceRequest.has_value() and resistanceRequest.reset() instead of having a "no action constant". If you are agreeable, I will do this for the resistance, inclination and power requests, in a different PR.
There was a problem hiding this comment.
sorry for the delay @drmason789
the -20 in this case was copied by another project that I found so actually I didn't care about it. It could be that this bike can have also negative resistance (so it will make sense to use -100 for the resistance too). I didn't remember this.
and use resistanceRequest.has_value() and resistanceRequest.reset() instead of having a "no action constant". If you are agreeable, I will do this for the resistance, inclination and power requests, in a different PR.
yes sure! thanks!
- fixed test settings / watt gain - adjusted some wattsFromResistance functions to avoid test timeouts - temporarily use subset of resistance values for power conversion test to avoid timeouts
| if (requestResistance > 23) { | ||
| requestResistance = 23; |
There was a problem hiding this comment.
Unusual, the maxResistance function from the header returns 24.
There was a problem hiding this comment.
@drmason789 because the resistance levels are 24 but the bike wants it starting from 0. we can replace that 23 with max_resistance - 1
|
@cagnulein What is the intended difference between the functions wattsFromResistance and powerFromResistanceRequest? |
…Peloton resistance
|
@cagnulein Remember the resistance_t change? Would you be agreeable to me adding cadence_t, heartRate_t, power_t, pelotonResistance_t, inclination_t alongside it and propagating throughout the codebase, in a different PR? |
…ake Peloton conversions invert.
@drmason789 it was intented to use for renpho bikes but at the end powerFromResistanceRequest is a dead code |
@drmason789 yes sure! |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
@drmason789 i added erg tables for all the bikes with the new ergtable module. Maybe it could impact also this |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
To finish this PR, some thought will be needed about what it should test.
I have defined the erginterface abstract class, which acts as an interface for the powerFromResistance and resistanceFromPower functionality offered by the bike class. Currently the bikeergfunctions class implements this for bikes, by setting the bike's current cadence, using the corresponding function to get the resistance or power, then restoring the bike's original cadence.
The interface supports optionally defined (min, max) x (cadence, resistance) representing the domain the bike knows about so the tests adapt to the boundaries provided, where provided.
What is tested?
What is not tested?