Book request: remove deprecated AsyncTask#326
Open
wmergenthal wants to merge 3 commits intoBloomBooks:masterfrom
Open
Book request: remove deprecated AsyncTask#326wmergenthal wants to merge 3 commits intoBloomBooks:masterfrom
wmergenthal wants to merge 3 commits intoBloomBooks:masterfrom
Conversation
This change is not motivated by a failure I or anyone else has observed. But using deprecated code is not good long term, so this change replaces AsyncTask (deprecated in API level 30) with a Socket. This also has the minor benefit of reducing the thread count by 1. Some debug output is still present. I will remove it before making a PR.
Remove many debugs, modify others. Works on wired connection at home, next is to test on Wi-Fi at JAARS.
andrew-polk
reviewed
Jun 30, 2025
Contributor
andrew-polk
left a comment
andrew-polk
left a comment
There was a problem hiding this comment.
Looks good overall.
I think I'll have @JohnThomson take a look because
- Though I'm sure it has faded far into distant memory, I think he probably wrote this originally and might have an idea why we make it asynchronous in the first place.
- He has a better head for threading than me.
app/src/main/java/org/sil/bloom/reader/wifi/NewBookListenerService.java
Outdated
Show resolved
Hide resolved
Remove comment referencing 'SendMessage', a class being removed.
andrew-polk
approved these changes
Jul 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have not observed a failure with Reader sending a book request to Desktop, but the mechanism that does this extends a class (AsyncTask) that is deprecated as of API level 30. This changeset offers an alternative implementation. It's mostly the same stuff - UDP socket and JSON book request - but is just lifted out of the extended AsyncTask class. I could not see the need for the extra layer and extra thread, as the main thread does not need to resume listening for adverts after making a book request.
I left in some debug output code to log the content of the book advert received from Desktop. It is commented out. I have found this very useful, but it should probably be deleted in production code. Perhaps the same will be true of the handful of other lines, still uncommented, showing the progress of an advert's reception and processing. But I would encourage that they be retained if possible.
This change is