Improve camera tile add/remove event handling #3954
Merged
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.
Summary
Following reports by @dshokouhi in chats and on Discord of camera tiles resetting unexpectedly or not being removed from the app, I tested adding/removing camera tiles again.
For camera tiles resetting, this can be caused by
onTileAddEvent
oronTileRemoveEvent
. This PR updatesonTileAddEvent
to not overwrite tile data if it already exists, hopefully fixing this (remove+add is annoying, the app would have to add soft delete).For camera tiles sticking around, during testing I noticed that the system was often destroying the service almost immediately after the add/remove events were sent. As a result, code started in the service's coroutine was cancelled before completing, and a camera tile wasn't added to the database or not removed from the database. I could semi-reliably get it to destroy the service within several milliseconds of adding/removing.
To prevent this, making sure the database work is completed before completing the callback function seems to be the only reliable way (
runBlocking
). This PR changes it for the camera tiles, during testing it still finishes in <100ms and the app isn't visible at the time (system plays animation) so blocking isn't an issue.Screenshots
n/a
Link to pull request in Documentation repository
n/a
Any other notes
If accepted I intend to update other tile services to do the same later (there haven't been any complaints yet, but they use other storage which may behave differently on cancellation).