-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
G6 Transmitter end of life reminder #2191
Conversation
@@ -168,8 +175,23 @@ private void startSensorOrAskForG6Code() { | |||
final int cap = 20; | |||
if (Ob1G5CollectionService.usingCollector() && Ob1G5StateMachine.usingG6()) { | |||
if (JoH.pratelimit("dex-stop-start", cap)) { | |||
int TX_dys = DexTimeKeeper.getTransmitterAgeInDays(getTransmitterID()); | |||
PersistentStore.setBoolean(TX_EOL, false); |
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.
Don't use persistent store for temporary data. If you don't want to pass parameters then you can use the fast store like FastStore.getInstance().putL("blah",1);
and retrieve with FastStore.getInstance().get("blah");
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.
Thanks for teaching me yet one more thing. FastStore? Awesome
My concern with this PR was something else though. May be I was wrong.
I thought I was supposed to use the same variables in different classes. So, I thought this was a mistake I had made:
#2160
In that PR, I already have a case that creates different branches depending on the number of days. I thought I was supposed to do that only once and then use the same variables everywhere instead of having the case statement over and over.
So, I thought may be I was supposed to go back to that PR and make changes to it such that I can use the same variables.
Are you saying that if I make the change you have suggested here, you are OK with everything else?
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.
I always have difficulty resolving strings conflicts. My PRs that add strings always end up having conflicts and I don't know how to resolve them with github. I have to use Git. But, I prefer to use github because I don't have experience with git.
So, I have to close this an open a new PR. I am OK doing that. But, I feel bad every time I do that because the feedback already provided ends up being in the PR that I close.
@jamorham Those variables are passed to [G6EndOfLifeDialog.java], the new file containing the dialog.
Considering the variables are passed to another file, can I use FastStore?
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.
You can pass the variables to the dialog method if that makes more sense than using FastStore
You want to avoid duplicating code so if you evaluate variables once then where possible reuse these.
You could pass a Runnable to your dialog method and create a callback like that. I'm sure how complicated it needs to be.
I don't find this PR that easy to read but it looks like it should work and implement the desired functionality. I just don't want PersistentStore
used for temporary data because there is an overhead with that.
String conflicts just add yours in the middle of the file instead of at the end and have greatest chance not to get conflicts. |
This did it: #2343 |
The following is from here: #2143 (comment)
I think we should highlight in red when transmitter days >= 100 on system status for transmitters firmwares which wont start after that. We should perhaps mark it orange within 10 (or more) days before expiry to give a heads up on the status screen as well.
on start sensor we should display a warning dialog that sensor start is likely to fail if the same conditions are met and allow the user to cancel sensor start.
I'll leave the exact nature of when to warn the user about transmitter expiry via notification to people who suffer from this problem more. Here it works on a subscription model so transmitters are sent to you ahead of time with sufficient to cover the entire subscription unless you want to buy more at an eye watering additional cost. I can see a use case though if someone were, for example, going on a trip and would be away from home only to find they can't start their next sensor...
1 has been addressed already. 233eb89
This PR addresses 2.
There are newcomers to G6 who insert a new sensor and start and after it fails, ask why, and we find out that the Transmitter Days on system status page has reached maximum. At that point, if they don't know how to remove the transmitter from the sensor to replace it with a new transmitter, they will lose a brand new transmitter.
This PR provides a reminder in the form of a dialog at the time of starting a sensor only for the last 3 or 4 sensors. This way, it will be less likely that the user would overlook the number of days on transmitter until it fails.
The following image shows the behavior for a transmitter that has already reached the maximum number of days for a standard Firefly.
