-
-
Notifications
You must be signed in to change notification settings - Fork 48
Marc roy #711
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
Marc roy #711
Changes from 3 commits
48a00fb
143c791
48914a7
42de858
711cf25
25ba34b
7a4d78d
2b6b4a5
b09376b
e64c726
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -121,6 +121,7 @@ void ErgMode::runERG() { | |||||||||||||||||||
| void ErgMode::computeErg() { | ||||||||||||||||||||
| Measurement newWatts = rtConfig->watts; | ||||||||||||||||||||
| int newCadence = rtConfig->cad.getValue(); | ||||||||||||||||||||
| int32_t result = RETURN_ERROR; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| bool isUserSpinning = this->_userIsSpinning(newCadence, ss2k->getCurrentPosition()); | ||||||||||||||||||||
| if (!isUserSpinning) { | ||||||||||||||||||||
|
|
@@ -141,29 +142,29 @@ void ErgMode::computeErg() { | |||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| #ifdef ERG_MODE_USE_POWER_TABLE | ||||||||||||||||||||
| // SetPoint changed | ||||||||||||||||||||
| #ifdef ERG_MODE_USE_PID | ||||||||||||||||||||
| if (abs(this->setPoint - newWatts.getTarget()) > ERG_MODE_PID_WINDOW && rtConfig->getHomed()) { | ||||||||||||||||||||
| #endif | ||||||||||||||||||||
| _setPointChangeState(newCadence, newWatts); | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| #ifdef ERG_MODE_USE_PID | ||||||||||||||||||||
| result = _setPointChangeState(newCadence, newWatts); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| #endif | ||||||||||||||||||||
| #endif | ||||||||||||||||||||
|
|
||||||||||||||||||||
| #ifdef ERG_MODE_USE_PID | ||||||||||||||||||||
| // Setpoint unchanged | ||||||||||||||||||||
| _inSetpointState(newCadence, newWatts); | ||||||||||||||||||||
| if (result == INT32_MIN) { | ||||||||||||||||||||
| result = _inSetpointState(newCadence, newWatts); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| #endif | ||||||||||||||||||||
| _updateValues(newCadence, newWatts, result); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| void ErgMode::_setPointChangeState(int newCadence, Measurement& newWatts) { | ||||||||||||||||||||
| // It's better to undershoot increasing watts and overshoot decreasing watts, so lets set the lookup target to the nearest side of ERG_MODE_PID_WINDOW | ||||||||||||||||||||
| int adjustedTarget = newWatts.getTarget() >= newWatts.getValue() ? newWatts.getTarget() - ERG_MODE_PID_WINDOW : newWatts.getTarget() + ERG_MODE_PID_WINDOW; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| int32_t ErgMode::_setPointChangeState(int newCadence, Measurement& newWatts) { | ||||||||||||||||||||
| // It's better to undershoot increasing watts and overshoot decreasing watts, so lets set the lookup target to the nearest side of POWERTABLE_WATT_INCREMENT | ||||||||||||||||||||
| int adjustedTarget = newWatts.getTarget() >= newWatts.getValue() ? newWatts.getTarget() - POWERTABLE_WATT_INCREMENT : newWatts.getTarget() + POWERTABLE_WATT_INCREMENT; | ||||||||||||||||||||
| int32_t tableResult = powerTable->lookup(adjustedTarget, newCadence); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // A lot of times this likes to undershoot going from High to low. Lets fix it. | ||||||||||||||||||||
|
Comment on lines
+160
to
+163
|
||||||||||||||||||||
| int adjustedTarget = newWatts.getTarget() >= newWatts.getValue() ? newWatts.getTarget() - POWERTABLE_WATT_INCREMENT : newWatts.getTarget() + POWERTABLE_WATT_INCREMENT; | |
| int32_t tableResult = powerTable->lookup(adjustedTarget, newCadence); | |
| // A lot of times this likes to undershoot going from High to low. Lets fix it. | |
| int lookupTarget = newWatts.getTarget() >= newWatts.getValue() ? newWatts.getTarget() - POWERTABLE_WATT_INCREMENT : newWatts.getTarget() + POWERTABLE_WATT_INCREMENT; | |
| int32_t tableResult = powerTable->lookup(lookupTarget, newCadence); | |
| // A lot of times this likes to undershoot going from High to low. Lets fix it for the comparison target. | |
| int adjustedTarget = lookupTarget; |
Copilot
AI
Dec 31, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition "tableResult < 0" will be true when tableResult equals RETURN_ERROR (which is INT32_MIN, a negative value). This results in a redundant check and assignment when tableResult is already RETURN_ERROR. Consider adding "&& tableResult != RETURN_ERROR" to the condition on line 181 to avoid this redundant operation.
| if (rtConfig->getHomed() && tableResult < 0) { | |
| if (rtConfig->getHomed() && tableResult < 0 && tableResult != RETURN_ERROR) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent spelling of the name "Marc Roy" vs "Mark Roy" in the changelog. Line 26 says "Mark Roy" while line 28 says "Marc Roy". These should be consistent.