-
Notifications
You must be signed in to change notification settings - Fork 1
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
Conversation
…at to the collection
There was a problem hiding this 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?
@texasRanger09 This should close #74 , so I'll go ahead and update that to link here and add your name etc |
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 |
@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. |
@okdistribute Any objections to merging this as-is and revisiting the subscription in #84 ? That would keep Android and Swift in sync for now |
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? |
Feel free to merge but yeah, it needs another look |
#94 will cover this as well, this can be closed after that PR is merged. |
fixes #73
part of #79
fixes #74