-
Notifications
You must be signed in to change notification settings - Fork 8
Android15 lenovo m10 #89
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
Conversation
@@ -1474,6 +1479,15 @@ private void startDFUFlash() { | |||
.setKeepBond(true) | |||
.setForeground(true) | |||
.setZip( getCachePathAppZip()); | |||
|
|||
// https://github.com/microbit-foundation/microbit-android/issues/82 | |||
// ? if ( Build.HARDWARE.equals("ums512_1h10")) { |
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.
Are you still consider this as an option? Or leaving in case we want to broaden it to other manufacturers/devices with the same hardware in future?
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'm not sure what is the best way to catch the required devices. I felt most comfortable with using the names, but I had seen this:
https://github.com/NordicSemiconductor/Android-DFU-Library/blob/94e8f9a8def016fab237d2d480698c3b3957e280/lib/dfu/src/main/java/no/nordicsemi/android/dfu/BaseDfuImpl.java#L745
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.
Interesting! I guess in the ideal world we'd test with a different vendor's ums512_1h10 (presumably that string came from your test device?) but that's not likely to be possible.
I'm happy going either way based on your judgement. If the slowdown isn't too bad maybe it's better to use the hardware and potentially fix more devices at the risk of perhaps making some working devices a bit slower? Please decide and merge one or the other.
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 just removed the comment. I didn't time them, but had the impression the M10 isn't as slow as my Samsung A8, but still I wouldn't want the slow down imposed on a device that was working before!
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.
Android 15 PR separately reviewed by @microbit-grace and approved.
The Lenovo workaround looks fine but the commented out line lacks context for why we've kept it so unclear whether it's intentional. Perhaps it could have an explanation if we want to keep it? Approving to unblock.
This is the same as #88, but with an extra commit to fix V2 DFU from Lenovo M10.