getditto / DittoAndroidTools

Diagnostic and Debugging Tools for Ditto Android SDK
MIT License
6 stars 1 forks source link

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

Closed texasRanger09 closed 5 months ago

texasRanger09 commented 6 months ago

fixes #73
part of #79 fixes #74

bplattenburg commented 5 months ago

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

okdistribute commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

Feel free to merge but yeah, it needs another look

bplattenburg commented 5 months ago

https://github.com/getditto/DittoAndroidTools/pull/94 will cover this as well, this can be closed after that PR is merged.