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

Don't subscribe to the heartbeat collection if not publishing heartbe… #85

Closed
wants to merge 1 commit into from

Conversation

texasRanger09
Copy link
Contributor

@texasRanger09 texasRanger09 commented May 31, 2024

fixes #73
part of #79
fixes #74

Copy link
Contributor

@zmarkan zmarkan left a comment

Choose a reason for hiding this comment

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

Is this because the subscription is only required for publishing heartbeats & it's not possible to publish without an ongoing subscription?

@bplattenburg
Copy link
Member

@texasRanger09 This should close #74 , so I'll go ahead and update that to link here and add your name etc

@okdistribute
Copy link
Collaborator

No a subscription is not required to publish heartbeats. I am not convinced yet that we should be subscribing to them as part of this tool since it's a side effect

@texasRanger09
Copy link
Contributor Author

I am not convinced yet that we should be subscribing to them as part of this tool since it's a side effect

@okdistribute We decided to include a subscription, so that if a device loses it's connectivity to the BP then it would still be able to multi-hop it's data to the BP. This allows us to gather more detailed information if a peer is having issues rather than just seeing that the peer is no longer connected.

@bplattenburg
Copy link
Member

bplattenburg commented Jun 7, 2024

@okdistribute Any objections to merging this as-is and revisiting the subscription in #84 ? That would keep Android and Swift in sync for now

@okdistribute
Copy link
Collaborator

I don't understand why it's called "publish to ditto collection" if it's a subscription - can you guys think through this feature a bit more?

@okdistribute
Copy link
Collaborator

Feel free to merge but yeah, it needs another look

@bplattenburg
Copy link
Member

#94 will cover this as well, this can be closed after that PR is merged.

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