-
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
Bugfix/xiaomi missed readings #3567
base: master
Are you sure you want to change the base?
Bugfix/xiaomi missed readings #3567
Conversation
…n-mode" to reduce missed readings and added some debug log outputs.
# Conflicts (solved): # app/src/main/java/com/eveningoutpost/dexdrip/services/Ob1G5CollectionService.java
…states into separate methods.
# Conflicts (solved): # app/src/main/java/com/eveningoutpost/dexdrip/services/Ob1G5CollectionService.java
# Conflicts (solved): # app/src/androidTest/java/com/eveningoutpost/dexdrip/HomeEspressoTest.java
Has this been tested? I have been using G7 with xDrip for 9 months with a Pixel 6a. I don't have unexpected missed readings. Of course when I walk away from the phone, I have missed readings. But, it recovers automatically shortly after I return. |
Are you sure you have disabled Xiaomi's aggressive battery optimization schemes when testing xDrip as is? |
Yes, I have been using these changes for a few weeks (since mid/end of May) without any problems with my Xiaomi POCO X6 5G (with MIUI 14.0.6 -> Android 13 based) and the G7. The problem before was that on my device (in <always_scan> mode, i.e. after I was back in range) at most every second value was read (often less). So there is a relatively big difference between a "direct" connect and the scan method. Therefore, in my opinion, it makes sense to deactivate this <always_scan> mode as soon as possible and (re)use the direct connection.
Yes, I did and it was really a challenge!^^ In this context I also wrote the instructions under [Troubleshooting - Xiaomi MIUI Optimizations](https://github.com/xdrip/xdrip_docs/blob/master/docs/troubleshoot/savings.md#xiaomi-miui-optimizations). |
I'm not the person who will have to approve this. So, please take my comments for whatever they are worth. I just put my phone (running the latest Nightly) in the microwave for more than 15 minutes to cause 3 consecutive missed readings. Then, I removed it and it reconnected. Looking at the data table, you can see that readings at 17:55, 18:00 and 18:05 are all backfills. And the following readings are direct collections with no backfill. This shows that xDrip reconnects with no issue. You can do the same using the latest Nightly and demonstrate how your phone malfunctions before your PR. Without that, it may be hard to convince someone that we need to change anything. |
Apparently, scanning for devices on a Pixel 6a behaves differently than on Xiaomi devices. The behavior of "skipped readings" after you are back in range (on Xiaomi devices) has already been noted and described in the discussion here: #3454 (see here) |
Thanks for helping improve xDrip for Xiaomi users. My concern with this PR is that it adds a new setting that everyone can see for something that seems to affect only one phone brand and only G7. The problem I see is that users make mistakes with the settings on the G5/G6/Dex1 Debug Settings page. The problem with this page is that there are some very critical settings on it. By having too many settings on it, it is possible that one setting will be set incorrectly by a user who is not very careful. Then, they will report it as a failure of xDrip. Will you consider making your changes conditional based on the manufacturer of the phone? xDrip/app/src/main/java/com/eveningoutpost/dexdrip/utils/framework/BuggySamsung.java Line 71 in f418d5b
|
The option <allow_scan_by_mac> was originally introduced (internally) by @jamorham and defines (for all phones and "Dexcom sensors") whether a MAC filter with the Bluetooth MAC address of the sensor should be used when scanning (as long as this MAC address is already known). Since this option is very useful in my opinion, I only made it accessible as a property via the user interface.
OK, I understand! For me it is absolutely no problem to move this menu to another place. My reasoning when placing this property was simply to keep related properties together. Since the different Bluetooth scan properties were already present at this point (such as <ob1_minimize_scanning> or <ob1_avoid_scanning>), this was (contextually) the right place in my opinion.
@Navid200: In your opinion, what would be a better place (contextually) for the property <ob1_allow_scan_by_mac>? |
I still don't see why a new setting is necessary. Samsung users already have a special setting that most of them forget to set and I have to tell them to enable special pairing workaround. I am not in favor of adding new settings, which will add to the possibility of such errors. Please lets wait for the lead developer's review. |
We have informed users that Xiaomi is a known problem: I'm looking at Xiaomi phones that run Android 14 and their prices are very high: The same retailer shows Pixel 6a for less than half the price: Pixel 6a currently runs Android 14 and it will upgrade to Android 15 when it goes public in 2-3 months. xDrip works fine as is. Why should we modify xDrip and complicate it for a phone manufacturer that we know does not follow protocol? I am sorry that you have a Xiaomi and it may seem to you that I have a grudge against you. I don't. I don't want this kind of publicity: I hope this to be an open and unbiased conversation. Can you help me understand why people buy Xiaomi phones knowing that the manufacturer does not care about apps that run in the background? |
I can't answer this question in general, but I think that your comparison is mixing up price ranges of smartphones. ;) In my case, my previous phone was a Huawei phone and xDrip+ worked on it for years with absolutely no issues. Therefore, my (perhaps somewhat naive ;) ) expectation was that it would work even better with a Xiaomi device.^^ And yes, now I know that the "optimizations" developed by Xiaomi in recent years are a massive pain in [...it is not allowed to write here what I'm currently thinking...] unless you know how to deactivate them (and have the chance to deactivate them). Since I deactivated the "MIUI optimizations" and implemented my changes, xDrip+ works absolutely reliably again on my Xiaomi device.
I understand your concerns. But isn't the quality of software precisely defined by the fact that it works reliably under difficult conditions on different devices? :) I suspect that the publicity could suffer more if certain smartphone manufacturers were to be required to use xDrip+. Can it not even be an additional unique selling point if xDrip+ works reliably even on Xiaomi devices? ;) |
@NiK27711 Can you describe the significant changes that you have implemented that you think improve Xiaomi reception? It isn't really clear from your code due to other structural changes. @Navid200 Xiaomi do make good phones, which are usually relatively inexpensive, but there are many non-standard changes made to the operating system, adverts appear in system apps and I have had random games install while the phone was idle. |
@jamorham: I think the key changes targeting Xiaomi devices are:
|
Can we remove the setting and have it enabled all the time? If this is a kill switch, can we put it under other misc options with the default set to enabled and agree that it will be removed in a year? I will remove it. |
The thing is, from my point of view, that in this context we are talking about the configuration or the way of using the class |
Mac filtered scan does not work reliably which is why it isn't used |
@NiK27711 can you identify a handset which exhibits the problem this PR is designed to solve? The last checks I did there was not a problem with xiaomi so long as power optimization was disabled for xDrip. After reboot battery optimization also reverted but xDrip will warn of that as soon as you open it. |
@jamorham: The problem occurred on my Xiaomi POCO X6 5G (with MIUI 14.0.6) even when battery optimization was disabled. :) |
These changes are intended to fix problems that, among other things, lead to "missed readings" when receiving G7 sensor data on Xiaomi devices (with OS MIUI 14 - see also Troubleshooting - Xiaomi MIUI Optimizations). In addition, the <always_scan> mode, which is automatically activated when necessary, can lead to frequent interruptions in receiving the values. For this reason, this mode will now be deactivated again as soon as possible in "normal operation".
To analyze and isolate problems of this kind, I have added additional debug logs to the . For the option <allow_scan_by_mac> introduced by @jamorham, I added an additional (menu) property <ob1_allow_scan_by_mac>.
In order to increase the readability of parts of the class , I've also moved the code for determining the different Bluetooth properties as well as the handling of the connection state changes into separate methods.