[WIP - do not merge] Libusb android #2309
Conversation
To allow efficient moving of downloaded dives from the download thread to the model, implement a general move function that moves table data. Instantiate that function for the dive and dive_site tables. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Currently, desktop and mobile are accessing the DownloadThread and the DiveImportedModel concurrently. This makes a big data flow mess. To achieve a more hierarchical data flow, start by making the DownloadThread a subobject of DiveImportedModel. Start the download by calling a function in DiveImportedModel. Route the finished signal through DiveImportedModel. Thus, the model can reload itself with the new data. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In DiveImportedModel::deleteDeselected(), unselected dives were deleted from the dive-table. But this left the model in an inconsistent state and the frontend was not informed of the missing dives. Fix this by invoking the appropriate beginRemoveRows()/ endRemoveRows() pairs. Move the functionality into its own function so that it can be reused by the desktop version. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In DownloadFromDCWidget::on_ok_clicked() deselected dives were directly deleted from the dive table, leaving DiveImportedModel in an inconsistent state. Use the function in DiveImportedModel instead. This also removes code duplication. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Add a comment that the passed-in dives are consumed. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
When importing dives, consume the tables from DiveImportedModel and not the DownloadThread. This appears more logical and avoids an inconsistent state of the DiveImportedModel: On import the tables would be reset, but the DiveImportedModel wasn't informed of that. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Clearing the table in the thread leaves the model in an inconsistent state. Don't do that. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The plan is to make the model the authoritative source of the imported dives. Therefore, access the number of downloaded dives from there. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
QML doesn't access these directly anymore but goes via the model. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
These are not passed through QML anymore. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Even though the returned dive is not const, the table is not changed, as it only contains pointers. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The DiveImportedModel and DownloadThread used the same table of dives and dive sites. This made it very hard to keep the model consistent: Every modification of the download thread would make the model inconsistent and could lead to memory corruption owing to dangling pointers. Therefore, keep a copy in the model. When updating the model, use move-semantics, i.e. move the data and reset the tables of the thread to zero elements. Since the DiveImportedModel and the DownloadThread are very tightly integrated, remove the accessor-functions of the dive and dive-site tables. They fulfilled no purpose whatsoever as they gave the same access-rights as a public field. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
DiveImportedModel::recordDives() called add_imported_dives(). But that actually consumes the dive and dive-site tables. Which in turn will lead to an inconsistent model. Properly reset the model by using the consumeTables() function. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This index was never set to anything else than 0. Might as well remove it. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This is redundant, as the actual size is stored in the dive table. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The download thread is not accessed directly by QML anymore. Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
On many devices the Screen property doesn't give us a manufacturer, model, or name. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Simply nice to have - even though it didn't help me track down the issue this time around. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
So even setups relying on the wrapper script can do faster rebuilds. This also cleans up a bug that made passing through the release parameter fail in the past, and removes overly verbose debugging output. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Different Android devices seem to have different patterns of throwing incorrect width information at us. This seems like a really bad hack, but for the ones I've seen so far this should give us the right width information. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We want the new default font in place before the QML engine is initialized. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
Somehow we need to be able to figure out which width information is correct. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
We get incorrect changes to a new screen width that causes us to try draw to a much larger screen than we actually have. Ignore those changes. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
There was a sad typo in commit eecca6a ("Mobile: replace model-reset by row-addition in DiveListModel::reload()"). Additionally, we need to also refresh the dive list so that the new dives are shown. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This only applies to those platforms where we build libusb from source, but critically that includes Android and v1.0.23 contains several Android updates that might get us closer to broader Android support for FTDI downloads. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This is based on patches from Anton Ludin and adds the new device scanning callback. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This adds all devices that Android reports back to us to the libusb device list. Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
|
Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off. That's something we need before your Pull Request can be merged. Please see our contributing guide. |
So we find the device that is connected (yay), we are able to open it (extra yay), but then we fail in |
|
Good work.
We obviously have opened the device and is talking to it, but we can't really talk to it. This is probably due to the os_priv bits in the struct libusb_device being random data. I guess we need to try to call initialize_device in android_enumerate_device. |
|
That call to |
|
Placed a call to |
|
Several more hours of trying to find a way to get libusb to be able to access config and descriptors. I must be missing something very fundamental. We get an fd that seems valid, but then all the reads go nowhere. No errors, just zero bytes back. |
|
sorry to be a pain @dirkhh but i'm trying to build from this branch, to hopefully get as far as you have done above but I don't think it's seeing the new "libusb_set_android_scan_devices_callback" function that was added. I'm guessing the subsequent errors would be because of the first Are there any other changes that would be needed to get it to run as far as your example logs show? it's very likely that i won't be able to help but i'd love to try - with that in mind, if you don't have time for this I understand! |
|
That function is added here 3d52b5a specifically https://github.com/Subsurface-divelog/subsurface/blob/abda7296c9102a65d00761c777539e11717515a7/packaging/android/patches/libusb-android.patch#L42 So my guess would be that the patch failed earlier, or something went wrong with making sure that you actually use the updated libusb |
|
thanks @dirkhh , it was the patch - i think a previous build run from master branch had already patched in libusb_set_android_open_callback so the check in the build script thought it didn't need to patch. |
|
I tried adding a few bits to see how far i could get (spoiler - not very) I thought I'd try and put in some of the same calls that you use when you run the get_usb_fd function, in the hope that i could get the libusb device "object", then use that to populate the values for the enumerate call - as you mentioned thinking that you need to get the device address. After: I put in: I don't think is particularly helpful as it's not working, but thought it might be a different way of getting the required info.........even if getting the device just so we can get the details needed to get the device later seems odd |
|
@StephenGoodall - I am so hoping that you are still looking at this. Or someone else (nudge @glance- hint). I SO desperately want someone to figure this out and fix it... |
|
@dirkhh I've been on holiday and catching up on work but it's still in my thoughts! When I get more time I'll have another try at poking it to see if I can get any further..... I don't understand c++ though so I'm not too hopeful, I can be quite persistent though so I'll keep plugging away! |
|
I would be surprised if understanding of C++ was critical here. It's the understanding of the interaction of two C libraries with the serial device that we get back from Java. |
|
this is no longer useful. closing |
Describe the pull request:
Pull request long description:
This looks like a huge PR, but that's because it includes #2305
Only the last three commits are interesting
Changes made:
Related issues:
Many, many, many - all the ones stating that you can't download from FTDI dive computers on Android.
Of course, you STILL can't download from them, not even on the Android phones were previously this worked. So it its current state this PR makes it worse. But it feels like I'm close.
Additional information:
Release note:
Yet to come
Documentation change:
Mentions:
@glance- please take a look and help me figure out what's wrong. I'll add a
adb logcatthat shows how far we get as a comment in a moment.