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

rename smbIsAlwaysOff to smbIsScheduledOff #20

Merged
merged 1 commit into from
May 1, 2024

Conversation

MikePlante1
Copy link
Contributor

@MikePlante1 MikePlante1 commented Apr 13, 2024

More importantly, it reverts the swapping of smbIsOff and smbIsAlwaysOffsmbIsScheduledOff

Must be merged in tandem with nightscout/Trio#120

@mountrcg
Copy link
Contributor

mountrcg commented Apr 16, 2024

To me it look there is only small things off, pun intended. In OiAPS the two variables should be switched:

Text Variable
Disable SMB's smbIsOff
Schedule when SMB's are off smbIsScheduledOff

And if smbIsScheduledOff gets toggled to true, the smbIsOff must toggle false. If that is done the oiref code should be fine.

… edited to reflect variable name change

@MikePlante1
Copy link
Contributor Author

To me it look there is only small things off, pun intended. In OiAPS the two variables should be switched:

Text Variable
Disable SMB's smbIsAlwaysOff
Schedule when SMB's are off smbIsOff
And if smbIsOff get toggled to true, the smbIsAlwaysOff should become false. If that is done the oiref code should be fine.

Yes, that would make more sense. I’ll put together a PR for the Oi repo instead and close this one.

@MikePlante1
Copy link
Contributor Author

@mountrcg If we're changing them anyway, I think smbIsScheduledOff would be a better name than smbIsOff

@bjornoleh
Copy link
Contributor

@mountrcg If we're changing them anyway, I think smbIsScheduledOff would be a better name than smbIsOff

Agreed, that's a more descriptive name for that variable. I'll take another look later if you are changing this PR or creating another in OiAPS?

@MikePlante1
Copy link
Contributor Author

MikePlante1 commented Apr 16, 2024

@bjornoleh It will require a PR to the Oi repo, but I'm not sure if I'll open a new one or just modify this one to match the updated variable names.

EDIT: I don't think I'll get to it today, but will definitely get it done by tomorrow (4/17).

@MikePlante1 MikePlante1 changed the title switch smbIsAlwaysOff with smbIsOff rename smbIsAlwaysOff to smbIsScheduledOff Apr 17, 2024
@avouspierre avouspierre added the enhancement New feature or request label Apr 28, 2024
@avouspierre avouspierre removed their assignment Apr 28, 2024
@JeremyStorring
Copy link
Contributor

Everything here looks good to me. I agree with the variable renames

@bjornoleh
Copy link
Contributor

I agree that this looks good, and I have successfully tested this in OiAPS too.

@bjornoleh bjornoleh merged commit 5774155 into nightscout:dev May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants