Fix HR PID race condition with training program speed changes

[Qhilm]

Description

This PR fixes multiple race conditions where the HR PID controller and inclination-compensation logic would override explicit training programme speed/inclination choices, causing speed oscillations and programme instability.

Problem

When a training programme transitions between rows (e.g., changing speed and/or inclination), competing systems can overwrite the programme's explicit speed choices:

1. HR PID race condition (original issue): The HR PID controller reads the current speed, calculates an adjustment, then fires the change. If the training programme changed speed between these steps, the HR PID overwrites the new speed with a stale calculation.
2. Inclination-compensation race condition (newly identified): When inclination changes, the wattage-preservation logic recalculates speed to maintain constant effort. It uses lastWattage from the previous update cycle (when the treadmill was still at the old incline), causing large, unwanted speed corrections.
   Observed in logs: At 16:15:48, the training programme sets speed to 11.5 km/h at incline 1%. Two seconds later, the inclination-compensation overwrites this to 8.5 km/h using stale wattage data, defeating the programme's explicit speed choice.

Solution

Commit 311a973: "Fix HR PID race condition with training program speed changes"

• Track when training program changes speed via lastTrainingProgramSpeedChange timestamp
• Skip HR PID adjustments for delta seconds after a training programme speed change
• Applies to both zone-based and min/max HR PID modes
• Allows body to respond to new speed before HR PID intervenes

Testing

This fix has been validated against logs from interval training with 6 speed transitions per cycle (12 transitions total), showing:

• No unwanted speed overrides during forcespeed rows
• No HR PID interference during training program transitions
• HR PID still functions normally in HR-zone-driven rows (where it should)

Backward Compatibility

• Existing setting trainprogram_pid_ignore_inclination continues to work as before (can globally disable inclination-compensation)
• New logic only affects behaviour during training programs with explicit forcespeed rows
• No behaviour change for manual UI speed/inclination controls
• No behaviour change for Zwift/external app control

[cagnulein]

Thanks do you need the build for ios for this?

[Qhilm]

Currently I followed your recommendation and added short non-HR guide rows in-between, so it's not urgent, but if it's possible, sure.

[cagnulein]

Ok I will try to build it ASAP

[Qhilm]

I edited the PR. Indentation was messed up.

[cagnulein]

it's building, could you please send me an email to roberto.viola83 at gmail.com indicating this so i can send you the beta?

[Qhilm]

I just tested, it doesn't have any effect somehow. Are you certain build 1348 includes this commit?

I have configured my training programm (see below) to have a short row to set the speed and then move on to an HR driven row, like you suggested, but even like this, the HR controller just overrides the speed of the new "static speed row".

Example:

Time	Event	Speed	HR
16:11:46	Trainprogram sets speed=7.5 (transition row)	7.5	70
16:11:51	Zone 2 row starts (minSpeed=6, maxSpeed=9, loopTimeHR=5)	7.5	~70
16:11:56	HR PID increases speed (+0.2)	7.7	--
16:12:01	HR PID increases speed (+0.2)	7.9	--
16:12:06	HR PID increases speed (+0.2)	8.1	--
16:12:11	HR PID increases speed (+0.2)	8.3	--
16:12:16	HR PID increases speed (+0.2)	8.5	--
16:12:21	HR PID increases speed (+0.2)	8.7	--
16:12:26	HR PID increases speed (+0.2)	8.9	--
16:15:46	Trainprogram sets speed=11.5 (next transition row)	11.5	116
16:15:48	HR PID immediately overrides to 8.5	8.5	116

<?xml version="1.0" encoding="UTF-8"?>
<rows>
    <!-- Warm-up walk -->
    <row duration="00:03:00" speed="5.5" inclination="0" forcespeed="1">
        <textevent timeoffset="10" message="HR 100-120 – Activez votre tronc."/>
    </row>
    <!-- Set initial speed before enabling HR zone control -->
    <row duration="00:00:05" speed="7.5" inclination="0" forcespeed="1">
    </row>
    <row duration="00:03:55" inclination="0" zonehr="2" looptimehr="5" minspeed="6.0" maxspeed="9.0">
        <textevent timeoffset="5" message="Zone 2"/>
    </row>
    <!-- Interval block: 6 x (90s hard / 90s recovery) -->
    <repeat times="6">
        <!-- Set initial speed before enabling HR zone control -->
        <row duration="00:00:05" speed="11.5" inclination="1" forcespeed="1">
        </row>
        <row duration="00:01:25" inclination="1" zonehr="4" looptimehr="5" minspeed="10.0" maxspeed="13.0">
            <textevent timeoffset="5" message="Zone 4"/>
        </row>
        <!-- Set initial speed before enabling HR zone control -->
        <row duration="00:00:05" speed="6.8" inclination="0" forcespeed="1">
        </row>
        <row duration="00:01:25" inclination="0" zonehr="2" looptimehr="5" minspeed="5.5" maxspeed="8.0">
            <textevent timeoffset="5" message="Zone 2"/>
        </row>
    </repeat>

    <!-- Set initial speed before enabling HR zone control -->
    <row duration="00:00:05" speed="8.5" inclination="1" forcespeed="1">
    </row>
    <row duration="00:04:55" inclination="1" zonehr="2" looptimehr="10" minspeed="7.0" maxspeed="10.0">
        <textevent timeoffset="5" message="Zone 2 – Bon travail sur les intervalles !"/>
    </row>

    <!-- Set initial speed before enabling HR zone control -->
    <row duration="00:00:05" speed="7.0" inclination="0" forcespeed="1">
    </row>
    <row duration="00:02:55" inclination="0" zonehr="1" looptimehr="10" minspeed="5.0" maxspeed="9.0">
        <textevent timeoffset="5" message="Zone 1"/>
    </row>
    <row duration="00:02:00" speed="5.0" inclination="0" forcespeed="1">
        <textevent timeoffset="5" message="Terminé ! Roulez vos fessiers au rouleau de mousse."/>
    </row>
</rows>

debug-Fr__Apr__24_16_08_19_2026.log.zip

[cagnulein]

Yes it does

You can put the debug log to claude to review the PR, I didn't check your code

[Qhilm]

Looks like the issue this time could be the inclination-compensation firing and overwriting whatever the new interval has set.

I have enabled the "PID Ignore Inclination" setting and will re-test.

[Qhilm]

hmm, no, the "PID Ignore Inclination" setting only applies to HR driven rows. The issue I listed above happens during a forcespeed=1 row (no hrmin/hrmax), hence it wouldn't work here. I need something additional for rows with forcespeed=1.

Would you be open to disable inclination-compensation if forcespeed=1? That seems like the smallest-scale change to avoid the conflict. For HR driven rows, the setting "PID Ignore Inclination" should be used. I am not sure I fully understand the principle of the inclination-compensation and if disabling it when forcespeed=1 doesn't break some logic/paradigm.

If you are ok with this, I can add a commit to this PR.

[cagnulein]

@Qhilm check if you like my last commit, i didn't test it yet

[Qhilm]

I think this only works if the inclination change completes in under a second, if my understanding is correct. Otherwise we run into the issue again, because it fires every second. Correct?

I think my treadmill needs more than a second for a 1% change already. Meaning, after the 1s, inclination is still changing, and the compensation will start kicking in and changing the speed. This is almost only problematic for training programmes with large sudden speed changes, such as interval training. That's why I noticed this issue during my interval training programme.

The problem with my approach on the other hand is that it might change the behaviour of the app for existing XML training programmes. But from a logical point of view, I find it correct that forcespeed=1 means "I want this speed, don't compensate anything".

Overall I think a central "arbitrer/controller" for any changes in speed/inclination (and other factors on other fitness devices) is the right way to fix all these race conditions, but it's a very large change and a different discussion. For now, let's focus on this race condition here.

[cagnulein]

I guess you are right. What if we add initial speed and inclination values for the HR PID, automatically inherited from the previous training program row somehow?

What I mean is: the HR PID knows the current training program line. On the first call for a new row, it could check whether the previous row had a speed and/or inclination value. If so, it could use those as the starting point.

Does that make sense?

[Qhilm]

So we have two issues in this PR actually:

1. HR PID preventing treadmill to complete speed changes that take multiple seconds
2. inclination-compensation that does the same.

The current commits fix issue 1/ already, I am focusing on 2/ right now. Unless you want to revert the commits I already did?

For 2/, one solution is "PID Ignore Inclination". It solves the issue for HR driven rows.

For forcespeed=1 rows (which should never have hrmin/hrmax as well, as you taught me), a code change is also required. My suggestion is to disable inclination compensation entirely for rows with forcespeed=1, but I don't have insights in how disruptive this could be for existing users.

I am not clear which issue you want to address with this:

initial speed and inclination values for the HR PID, automatically inherited from the previous training program row.

What the logic should be, in my opinion:

1. If forcespeed=1, the treadmill must always be allowed to reach this speed first, even if it takes multiple seconds. The next modifications can then take place.

• An arbitrer could manage this: "for each speed change, continuously read actual treadmill speed and wait for treadmill to read last requested speed before making any other change. Subsequent changes must all base on the target speed, meaning no queuing." But this is high effort, large code change.
• Alternatively I have suggested disabling PID HR for 10s at the beginning of each row (previous commit on this PR)
• The remaining issue is inclination-compensation for non-HR-driven rows. My suggestion is to disabled inclination-compensation completely for such rows.
• HR PID in my view also should never request an absolute speed, only increments. There should be a calculateIncrement() function that calculates the increment based on how fare away HR is from the target HR and how fast it is currently changing. This is also a large change for a different PR.

[cagnulein]

Thanks for the clarification. It's better now :)

Ok so about the

For forcespeed=1 rows (which should never have hrmin/hrmax as well, as you taught me), a code change is also required. My suggestion is to disable inclination compensation entirely for rows with forcespeed=1, but I don't have insights in how disruptive this could be for existing users.

I don't understand why you are talking about forcespeed=1 rows where, your training program here #4553 (comment) , the issue was in the line

<row duration="00:01:25" inclination="1" zonehr="4" looptimehr="5" minspeed="10.0" maxspeed="13.0">
            <textevent timeoffset="5" message="Zone 4"/>
        </row>

do you mean you would like to add forcespeed="1" in your xml and handle this case in the PID HR to disable the "inclination-compensation" dinamically?

If so i guess it's safe for other users because i don't think others have used this scenario before (but i'm not aware of all the cases :) )

Let me know!

[Qhilm]

ah, ok. So maybe we can move this to a different PR. The current PR works, it prevent HR PID from blocking treadmill changes mid-change.

But I discovered it's not enough because inclination-compensation does exactly the same. Hence a second change is required for this, but strictly speaking it's a separate issue.

Should I move this to a different PR?

[cagnulein]

No we can keep these on this PR. Thanks

[Qhilm]

ok, so my suggestion, to fix issue 2/ (inclination-compensation preventing multi-seconds treadmill speed changes from completing) specifically for rows which already have forcespeed=1 was to disable it entirely for such rows.

For rows which are using HR PID (hrmax/hrmin), no need for a fix, as the setting "PID Ignore Inclination" already allows to disable the inclination-compensation for HR driven rows.

Are you ok with this approach? Then I would add a commit, you can build it and I will test it this week.

[cagnulein]

Yes let's try!

[cagnulein]

I checked the commit but if the PID Ignore Inclination was already enabled how this could fix the issue?

[Qhilm]

"PID Ignore Inclination" is only for rows with hrmin/hrmax, right? The commit above is for rows with forcespeed=1.

Unless I misunderstood what "PID ignore inclination" does?

[cagnulein]

The If condition where you added the "forcespeed" check already has the check of the "pid ignore inclination" so if the pid ignore inclination already jumps that condition. Do you agree?

[Qhilm]

You are right, my commit doesn't make sense.

Ok, let me test with the previous build you sent me and "PID Ignore Inclination" enabled and see if the interval training works first, before making any other change. If that works, we can revert the last two commits. If it does not work, I'll upload log and try to figure out why it didn't work.

[Qhilm]

ok, so the interval training works fine with:

• build 1348
• "PID Ignore Inclination" enabled

Hence I would revert the last two commits, it seems they are not needed, is that ok for you?

[cagnulein]

yes sure!

[cagnulein]

Thanks! Would you like to do some more session or could I merge it?

[Qhilm]

I think we're good, I undid all code changes since I tested, hence I would consider this branch tested.

I did a hard reset in order to only keep the first commit from the branch, it's cleaner, and I edited the original commit a bit: I had edited the .gitignore by mistake, I undid this.

You can merge I think.

[cagnulein]

Thanks!