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 resistanceLowersTarget #155

Merged
merged 1 commit into from
May 11, 2024

Conversation

MikePlante1
Copy link
Contributor

@MikePlante1 MikePlante1 commented May 6, 2024

Reproduced the error with current dev where target does not change even when resistance is detected and Resistance Lowers Target is toggled on.

After applying this PR, target was lowered as expected.

⚠️ WARNING: This fix will reset preferences again.

Addresses #152

Co-Authored-By: Robert <comps@diprobe.com>
@MikePlante1
Copy link
Contributor Author

DynamicISF is off, no temp targets or profile overrides are being used:
settings

Test before applying this PR:
prePR2
prePR1

Test after applying this PR:
postPR2
postPR2

@MikePlante1 MikePlante1 requested a review from dnzxy May 6, 2024 19:36
@bjornoleh
Copy link
Contributor

@mountrcg , does this look ok?

Copy link
Contributor

@bjornoleh bjornoleh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes look good, and it works as expected when testing. LGTM!🚀

Copy link
Contributor

@dnzxy dnzxy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build to and tested with a simulator. Code changes look sane and good to me.

@dnzxy dnzxy merged commit e393468 into nightscout:dev May 11, 2024
@mountrcg mountrcg requested review from bjornoleh and removed request for mountrcg and bjornoleh May 11, 2024 00:53
@mountrcg
Copy link
Contributor

Sry screwed up the reviews here.

@MikePlante1 MikePlante1 deleted the resistance_lowers_target branch May 11, 2024 00:56
@MikePlante1 MikePlante1 mentioned this pull request May 11, 2024
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.

4 participants