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.

Sign up for GitHub

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

Jump to bottom

Notify about DataBroker Disconnect #14

Merged
merged 7 commits into from
Oct 10, 2023
Merged

Notify about DataBroker Disconnect #14

merged 7 commits into from
Oct 10, 2023

Conversation

wba2hi
Copy link
Contributor

@wba2hi wba2hi commented Oct 5, 2023

Closes: #12

@Chrylo
Copy link
Contributor

Chrylo commented Oct 6, 2023

Getting the following error message in the logs after starting / disconnecting the data broker
Previous channel {0} was not shutdown properly!!! *~

@Chrylo
Copy link
Contributor

Chrylo commented Oct 6, 2023

The java mode does not seem to work with disconnecting

@wba2hi
Copy link
Contributor Author

wba2hi commented Oct 9, 2023

The java mode does not seem to work with disconnecting

Good catch postCreate was "to early" to register the listener, because the DataBrokerEngine is switched when toggling the Java-Compatibility mode. Registering now in connect() and unregistering in disconnect() resp. in onDestroy() in case the connection is not manually closed.

@wba2hi
Copy link
Contributor Author

wba2hi commented Oct 9, 2023

Getting the following error message in the logs after starting / disconnecting the data broker Previous channel {0} was not shutdown properly!!! *~

I can't reliably reproduce this issue, however I always got it once in a few tries.
I added some logic to always "shutdownNow" in case the Connection is closed, either by calling disconnect or when the state changes (unexpectedly) to anything else then ConnectivityState.SHUTDOWN. I couldn't see the issue since then, but unfortunately it's a devil's proof, as I could simply have been lucky with it.

@Chrylo
Copy link
Contributor

Chrylo commented Oct 9, 2023

LGTM

lukasmittag
lukasmittag approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@lukasmittag lukasmittag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. App works for me in Andorid Studio and an emulated Pixel 3a. What I tested:

  • Connect with one device and then calling disconnect and connecting again -> worked like a charm

@lukasmittag
Copy link
Contributor

@wba2hi could you do a manual rebase please?
image

@lukasmittag lukasmittag merged commit 617ff9f into eclipse-kuksa:main Oct 10, 2023
3 checks passed
@Chrylo Chrylo deleted the feature-12 branch October 11, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukasmittag lukasmittag lukasmittag approved these changes

@Chrylo Chrylo Chrylo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Notify about DataBroker Disconnect
3 participants
@wba2hi @Chrylo @lukasmittag