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

G7 cleanup #3220

Merged
merged 7 commits into from
Jan 5, 2024
Merged

G7 cleanup #3220

merged 7 commits into from
Jan 5, 2024

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented Dec 2, 2023

Correcting some notes to show G7 when G7 is being used.
Adding G7 to the menu items that correspond to G7 as well.

@Navid200
Copy link
Collaborator Author

Navid200 commented Dec 2, 2023

Screenshot_20231202-162601

Screenshot_20231202-162917

@Navid200 Navid200 marked this pull request as draft December 6, 2023 14:44
@Navid200 Navid200 marked this pull request as ready for review December 6, 2023 14:45
@Navid200
Copy link
Collaborator Author

Navid200 commented Dec 6, 2023

Screenshot_20231206-094356

This is tested.

Der-Schubi added a commit to Der-Schubi/xDrip that referenced this pull request Dec 11, 2023
Merge remote-tracking branch 'Navid200/Navid_2023_12_02' into schubi
@jamorham
Copy link
Collaborator

Overall I think this is useful but I don't really like the logic too much because I think you're going to potentially have problems in the transition between when the sensor is active before and after firmware information has been read. Can you modify isDeviceG7() to simply check for 4 digit tx_id ?

@Navid200
Copy link
Collaborator Author

Navid200 commented Dec 11, 2023

Can I open another PR to change "G6/Dex1 Support" to an array allowing 3 options (G5, G6/Dex1, or G7)?
Then, instead of looking for the number of characters in the transmitter ID, we can look at that setting.

I will test transitions and everything.
After that is merged, this PR will become simpler.

@Navid200
Copy link
Collaborator Author

Consider that in the future, we may have other devices that use a 4-digit transmitter ID.

@Navid200
Copy link
Collaborator Author

Navid200 commented Dec 12, 2023

I did the change.
If you approve adding a specific selector for G7 later, I will then replace these with conditionals on the selector.
But, for now, this should be good.

@Navid200
Copy link
Collaborator Author

Navid200 commented Dec 12, 2023

Screenshot_20231211-234405

@Navid200
Copy link
Collaborator Author

Screenshot_20231217-224608

@Navid200
Copy link
Collaborator Author

Will be tested before tomorrow.

@Navid200
Copy link
Collaborator Author

Tested with a G7.
Looks good.
Everything looks as shown in the screenshots above.

Copy link
Collaborator

@jamorham jamorham left a comment

Choose a reason for hiding this comment

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

Looks like it should work other than if AAPS is fussy about the data source name but if so hopefully that can be resolved.

@@ -143,6 +145,9 @@ boolean alwaysSendBattery() {
@Override
String getDeviceName() {
if (Ob1G5StateMachine.usingG6()) {
if (shortTxId()) { // If using G7
return "G7 Device";
Copy link

Choose a reason for hiding this comment

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

change to "G7 Transmitter";

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

G7 transmitter and sensor are together. There is no reason to call it a transmitter any longer. It is valid to call it a device to avoid calling it a transmitter.

@jamorham jamorham merged commit 896d7e0 into NightscoutFoundation:master Jan 5, 2024
1 check passed
@Navid200 Navid200 deleted the Navid_2023_12_02 branch January 5, 2024 21:54
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.

3 participants