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

Miband 5 and Amazfit band5 integration #1659

Closed

Conversation

bigdigital
Copy link
Contributor

This is an extension to previous miband integration. Now it brings support of Miband 5 and Amazfit Band5 devices. And many new features.
It was well tested. All details available on my blog
https://bigdigital.home.blog/2020/08/25/new-xdrip-now-supports-miband-5/

bigdigital and others added 28 commits May 1, 2020 10:31
…t updating if alarm is triggered)

Fix. Properly handle not initialized last treatment value ( the exception was handled by catch statement which interrupts watchface uploading )
Improvement. Decrease first update interval to 15 minutes when there no new data (data loss)
Add us/eu date format watchfaces
Display IoB on graph
…show

added watchafaces for miband5
added eu date format wathfaces
…ed to correctly position "no connection" layers )

Fix. Graph display changes. Display prediction line even when there no threatments available.
Fix. Graph display changes. Display hight and low lines under the bg line.
Fix. In rare cases, the BG data can stop sending (usually happen when Bluetooth connection lost while sending the nightmode )
Fix. Screen Brightness Timeout
…ight/low alarms ) (NightscoutFoundation#1425)

Improvement. An alarm would be received before watchface uploading (NightscoutFoundation#1425)
Improvement. More detailed alarm snooze confirmation text (NightscoutFoundation#1425)
Fix. The screen brightness does not restore on some phones NightscoutFoundation#1427
Removed. Send readings as a notification option. Now For MiBand 2 and 3 the readings would be sent as notification. For MiBand 4 and 5 would be used watchface uploading.
Removed. Call notification. For call notifications use MiFit or any thirdparty apk.
…ation that can control watch settings)

refactor MiBandType, to be more general for other watch models.
initial amazfit implementation
…ation that can control watch settings)

refactor MiBandType, to be more general for other watch models.
initial amazfit implementation
New. Send alarm missing message notification.
Fix. alarm hangs on “WAITING_USER_RESPONSE” state when nigtmode active
# Conflicts:
#	app/src/main/res/values-pt/strings-pt.xml
# Conflicts:
#	app/src/main/java/com/eveningoutpost/dexdrip/watch/miband/MiBandService.java
#	app/src/main/res/values-pt/strings-pt.xml
@nexero nexero mentioned this pull request Feb 26, 2021
@parapenT1sta
Copy link
Contributor

@jamorham Can you merge this, please? :-)

@jamorham
Copy link
Collaborator

I'm not worried about minor whitespace issues in the code, my main question here is can we get a brief description of the content of the .bin files and to confirm that they are safe to include in the project in terms of copyright etc.

Second to that is just that this doesn't introduce any issues for users not using the feature. I had a quick look through the code and it looks okay on that front (eg it just hooks in to the same places as the existing service)

If there are unused preference sections then they should be removed any any dynamic preference handling in Preferences should be done in a safe way with try/catch to ensure that any incompatible changes to the XML later don't bring down the whole app. I didn't fully check to see if that is the case but if that could also be confirmed.

@MasterPlexus
Copy link
Contributor

I'm not worried about minor whitespace issues in the code, my main question here is can we get a brief description of the content of the .bin files and to confirm that they are safe to include in the project in terms of copyright etc.

As I'm not the developer, but as I'm analyzing the bin's due to understanding process I could say those are watchface containers which includes XML/json informations for positions and triggers within a watchface. Also graphic files are included for building for example the values of the watch, or background. The bin's are compressed with specific algorithm and are produced normally with open source watchface editors. I did not see any copyright problems with them, except licensed graphic files are used (Wich does not look like in that way in my eyes)

@radugreta
Copy link

This is an extension to previous miband integration. Now it brings support of Miband 5 and Amazfit Band5 devices. And many new features.
It was well tested. All details available on my blog
https://bigdigital.home.blog/2020/08/25/new-xdrip-now-supports-miband-5/

Hello, Does it work olso with the mi band 6?

@salzmret
Copy link

salzmret commented Jun 7, 2021

This is an extension to previous miband integration. Now it brings support of Miband 5 and Amazfit Band5 devices. And many new features.
It was well tested. All details available on my blog
https://bigdigital.home.blog/2020/08/25/new-xdrip-now-supports-miband-5/

Hello, Does it work olso with the mi band 6?

No it does not. Even not with the mi band 5..

@emp-00
Copy link

emp-00 commented Jun 18, 2021

Any update? @bigdigital are you stlll working on "resolving the conflicts" ? Any solution on the horizon ?

@Navid200
Copy link
Collaborator

I don't see anything in the comments other than support and approval for this PR.
In other words, no one has said that we will not merge this.

However, none of the questions have been answered.
Either, the questions should be answered, or the developer can continue to provide support to the users in his fork.

Either way, I see no reason to keep the issues related to this open any longer.
For all I know, the developer may never answer the questions. In that case, what is the point in keeping the issues open?

@parapenT1sta
Copy link
Contributor

I don't see anything in the comments other than support and approval for this PR.
In other words, no one has said that we will not merge this.

However, none of the questions have been answered.
Either, the questions should be answered, or the developer can continue to provide support to the users in his fork.

Either way, I see no reason to keep the issues related to this open any longer.
For all I know, the developer may never answer the questions. In that case, what is the point in keeping the issues open?

I just made a comment on this personal blog, hopefully we can get a reply soon.

@tolot27
Copy link
Collaborator

tolot27 commented Jun 21, 2021

Artem is really busy but also working on the suggested changes.

@radugreta
Copy link

radugreta commented Jun 22, 2021 via email

@salzmret
Copy link

Any news?

@bigdigital
Copy link
Contributor Author

bigdigital commented Aug 30, 2021

my main question here is can we get a brief description of the content of the .bin files and to confirm that they are safe to include in the project in terms of copyright etc.

@jamorham The bin files contain the watcface itself. The watch face contains a set if images compressed with a specific algorithm and header which defines how interpreter and display these images would be on the device screen. The watcface design for all devices was created personally by me so there are no copyright issues. Before uploading this watchface to the device, the xdrip will inject the canvas.png image into watcface.bin file. The canvas.png acts as a dynamical part of the watcface which contains xdrip related information.

@savek-cc
Copy link

savek-cc commented Sep 2, 2021

@jamorham @tolot27 @bigdigital anything I can support to get this merged? I just bought a Mi Band 5 to be able to see my daughter's BG (after ditching the sony smartwatch) - so I either pull this into a local instance here -- or support in getting it merged into mainline ;).
On the other hand, it seems like all review comments have been resolved, so this looks good to go?

@tolot27
Copy link
Collaborator

tolot27 commented Sep 5, 2021

On the other hand, it seems like all review comments have been resolved, so this looks good to go?

No, some are still not commented and no commit was pushed containing the addressed comments.
@bigdigital You are newly finished with your rework. Thanks so far! Please continue and push your changes.

@salzmret
Copy link

Will it be merged soon?

@tolot27
Copy link
Collaborator

tolot27 commented Sep 23, 2021

Will it be merged soon?

Not as long as not all reviewer comments are addressed by @bigdigital.

@bigdigital
Copy link
Contributor Author

@tolot27 Sorry for not answering here for so long. I've done all suggestions mentioned here a couple of mont ago. But every time I try to merge the changes, there is something new happening that stops me(I also still have not configured a new github token authorisation to push local changes to the repository) . For now, this merge request is too far compared to my current branch. With the latest changes, has been added support for almost all amazfit devices, and now, miband6 support was added. All these things takes too much code and resources, not related to the healthy application in any way. With the the latest changes also were added an Android NDK depedency in order to support a specific encryption required in the new authentication protocol . I don't this it would be a good idea to leave all those things in the xdrip project. I'm thinking about moving miband and amazfit devices support to the separate application and use inter-app communications to transfer data between applications. I think this should leave a clear code in the xdrip and there would be no need to merge latest changes into xdrip so frequently. What do you think?

@tzachi-dar
Copy link
Contributor

This should be a good idea.
I think that before implementing this, we should find out what are the interfaces that are needed from xDrip.
Right now, xDrip already has a few interfaces that can be used without any change.
For example there are broadcasts that reveal the latest BG (push notifications), and there is a web interface that allows the watch to get historic BG (and more).
Using existing interfaces will make your deployment faster and will allow all people to use your program.

HTH
Tzachi

@bigdigital
Copy link
Contributor Author

Yes, I also think that moving miband related stuff to the separate application would be the best approach to support miband devices in xdrip app. In such cases the users can receive updates for xdrip and miband/amazfit related features much faster. At the moment the miband related resources already taking 2.5 MB of data, and it would grow with the new devices. Also in such case the xdrip stability would be much improved, since some unpredinced errors would not crush the main healthy application (although this never happens with my code). Regarding inter-app communication , I'm thinking about using amazfitcommunication protocol. It is already present in xdrip. The Amazfitservice class should do almost all things that need for interapp communication. But maybe it should be a little extended, especially for graph display configuration. I'm thinking that it would be better to draw graph on xdrip side and simply return a serialized ready to use image, than transferring a huge massive of graph-related data.

@tolot27
Copy link
Collaborator

tolot27 commented Nov 27, 2021

I'm thinking about moving miband and amazfit devices support to the separate application and use inter-app communications to transfer data between applications. I think this should leave a clear code in the xdrip and there would be no need to merge latest changes into xdrip so frequently.

That's IMHO a good idea. Beforehand, we should clearly define an extension/plugin interface for IPC, maybe based on Amazfitservice. I'd like to see it more generic to use it for future extensions as well. But we can go step by step.

On the other side, we can also find a way to improve the review and merge process. We need this anyway.

@bigdigital Are you still listening in the dev gitter room? If yes, we can continue our discussion there.

Please close this PR if you not work on it anymore. Thanks!

@tolot27
Copy link
Collaborator

tolot27 commented Dec 3, 2021

I'm closing this as discussed. @bigdigital Please let me know if you need support with the integration.

@tolot27 tolot27 closed this Dec 3, 2021
@tolot27 tolot27 added the invalid label Dec 3, 2021
@Navid200
Copy link
Collaborator

@bigdigital

Yes, I also think that moving miband related stuff to the separate application would be the best approach to support miband devices in xdrip app.

Would you please tell me if there is anything I can do to help with this?
I would love to have your work included so that users can take advantage of it the easiest way. I hate to keep telling people that Mi Band 5 is not supported by the official version.

@bigdigital
Copy link
Contributor Author

@Navid200 For now i work on prototyping a separate application. I hope i could spend more time on this on the holidays.

@Navid200
Copy link
Collaborator

@bigdigital I know. That will be wonderful.
After that is accomplished, you can keep updating the separate application to address the needs of the Mi Band users and add more updates as they become necessary.
But, you can do all of that without having to submit any PRs to xDrip. So, we can maintain xDrip as needed. And the user community will get support. It's a win-win-win in my opinion.
It was a brilliant idea. Thanks for that. Let's hope we can get it done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mi band 5 + xdrip