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.

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

removing event bus + improving listeners #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jashan1498
Copy link

  • removing event bus library
  • cleaning up listener logic
  • updating app side logic after removing event bus

- cleaning up listener logic
- updating app side logic after removing event bus
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@KhushbuShah25
Copy link
Contributor

Hi @jashan1498 , Thanks for your contribution. We will check and update the code.

@jashan1498
Copy link
Author

Hi @jashan1498 , Thanks for your contribution. We will check and update the code.

Hey @KhushbuShah25 @pedrominatel any update on this ? since its an urgent change that we need .. if there is some change required please let me know .. and can you also share the release timeline i.e when can we expect this to be merged and released

@RDrelec
Copy link

RDrelec commented Dec 16, 2024

Hi @KhushbuShah25 ,

This PR would be a great enhancement for us as well. Can you provide an update about it please ?

@KhushbuShah25
Copy link
Contributor

Hi @jashan1498,

Apologies for the delay. I was occupied with other tasks.
I have checked changes and tested provisioning library and apps.
Changes look good to me. But I found some issues while testing.
Here are my observations :

  • Manual provisioning flow stopped working (mostly due to not getting event in manual provisioning screen).
  • Also existing onEvent method code runs on Main thread which needs to be taken care by some other way like executing onEvent method statements in runOnUiThread(). Because displaying alert dialog (popup) for any error is not working for disconnection or connection failure events.
  • Remove listener is missing when user presses back (onBackPressed()) , due to which whenever event occurs, app is receiving callback on older instance (previous screen instance which no longer exists) and it crashes.
  • The changes are not backward compatible and so, if existing customers update the library for some other reasons, they would also have to make changes in many screens (provisioning flow screens) , which we would want to avoid.

Could you please address the mentioned issues and update this PR accordingly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants