G7 Classic Status page cleanup #3246
Conversation
|
So in principle this is a good change I think but I would like you to use some better helper methods to declutter things and avoid duplication. I will make a change for Additionally I will include support for |
|
Will be tested before tomorrow. |
|
Tested. Looks OK. |
|
I have made the change you asked for and I have tested it. |
| public boolean using_g7() { // True if we are using G7 | ||
| if (DexCollectionType.getDexCollectionType() == DexCollectionType.DexcomG5 && Pref.getBooleanDefaultFalse("using_g6") && shortTxId()) { // If using G7 | ||
| return true; | ||
| } | ||
| return false; |
There was a problem hiding this comment.
return DexCollectionType.getDexCollectionType() == DexCollectionType.DexcomG5 && Pref.getBooleanDefaultFalse("using_g6") && shortTxId()
There was a problem hiding this comment.
Thanks a lot. Fixing it in a minute.
| @@ -12,4 +12,6 @@ public interface MicroStatus { | |||
|
|
|||
| boolean xmitterBattery(); | |||
|
|
|||
| boolean using_g7(); // True when using G7 | |||
There was a problem hiding this comment.
not java style method naming. Java style method naming is camel case
also use java doc for "True when using G7"
| public boolean bluetooth() { | ||
| return DexCollectionType.hasBluetooth(); | ||
| public boolean bluetooth() { // Dexcom with Bluetooth except G7 | ||
| return DexCollectionType.hasBluetooth() && !using_g7(); |
There was a problem hiding this comment.
not sure if you are braking logic for other sensors with the extra check
There was a problem hiding this comment.
I don't know what you mean by that. Please explain.
There was a problem hiding this comment.
I see what you mean now.
First, let me thank you for spending the time to review.
Please let me explain this concern.
There are only two possibilities under all circumstances. Either we are using G7 or not.
If we are not using G7, what is being anded is true. Anding true with anything changes nothing. Therefore, nothing changes with respect to other sensors.
If we are using G7, the returned value becomes false, which removes three lines from the status page.
There was a problem hiding this comment.
Wow! You have a point. There is a third case of the value being unknown. I need to make sure if the value is unknown, it doesn't break anything. Thanks so much.
There was a problem hiding this comment.
@DiBochev Is it OK now?
|
Tested again after the syntax correction. |
|
I'm now using ShortTxId. Is anything else needed? |
Merge remote-tracking branch 'Navid200/Navid_2023_12_18' into schubi
| try { // True if we are using G7 | ||
| return DexCollectionType.getDexCollectionType() == DexCollectionType.DexcomG5 && Pref.getBooleanDefaultFalse("using_g6") && shortTxId(); | ||
| } catch (Exception e) { // If there are any unknowns, show that we are not using G7 | ||
| return false; |
There was a problem hiding this comment.
just a suggestion maybe it is good idea to add debug log here
|
We now have a best collector option of of G7. I have tested this after this last change with both G7 and G6. There is no change to the classic status page of G6.
|
|
The option to forget device does not work for G7. And that is why it is important to remove it because everyone is not aware that it does not work: |
|
@jamorham |
There are three items on the classic status page that are not useful for G7.
The reason is that the G7 Bluetooth name is not related to the transmitter ID as it was with G6.
How many people are tapping on forget device thinking that they are unpairing the transmitter?
I did!
If some day, we can figure out a workaround, we can bring them back.
But, for now, please approve removing them.