Companion: Improved Contact Management #1379
Conversation
slot overwrite logic can now safely use contact.lastmod to find oldest contact for overwrite
|
Thanks for the great work! I'm keen to get this merged in. Lots of users have been asking for this one. Full support has been added to the mobile app, and the functionality is now live on the web app for anyone wanting to test against this PR. Will leave for @ripplebiz to do another code review :) 
|
| uint8_t buzzer_quiet; | ||
| uint8_t gps_enabled; // GPS enabled flag (0=disabled, 1=enabled) | ||
| uint32_t gps_interval; // GPS read interval in seconds | ||
| uint8_t autoadd_config; // bitmask for auto-add contacts config |
There was a problem hiding this comment.
The new AUTO_ADD_ bits could be shifted by 1 (to the left) and could use the unused bits in the existing manual_add_contacts byte.
Prior to this PR, the manual_add_contacts byte is always accessed with a bitmask (of 1), so it's safe for legacy installs out there.
So, then you won't need the extra CMD_GET_ and CMD_SET_ (and the new _RESP)
I'd also renamed the manual_add_contacts member to something else as it would have a new meaning.
There was a problem hiding this comment.
I actually considered reusing manual_add_contacts that way but I decided against it because of how much other unrelated stuff is already in CMD_SET_OTHER_PARAMS, and querying it back via CMD_APP_START/RESP_CODE_SELF_INFO is already getting to be a bit messy with the other bits and pieces that are in that RESP.
The other thing was that I was unsure how clients would respond to reusing manual_add_contacts in this way. I ended up discussing it with @liamcottle at some length and we came to the conclusion that this is the cleanest way to go about it. From a separation of concerns viewpoint I think it makes sense for it to have it's own CMD/RESP codes too, because it doesn't really have anything to do with the other settings in CMD_SET_OTHER_PARAMS.
Using separate commands also means old clients can simply ignore the new codes rather than having to handle unexpected values in a response they already parse. It feels like continuing to stuff things into CMD_APP_START is complicating things unnecessarily when we could just use a unique CMD/RESP.
|
Looking good 👍 - but I and probably many others have the opposite problem 🙄 I think its very difficult to discover new user, because there are no auto avert in the software/firmware, both users have to be active on the same time to discover each other 😳 |
This should already be the case when you have auto-add turned on. Your companion node should store any new contact and they will be available when you next connect to the phone. That's not possible if your storage is full, but with this PR you would be able to turn on over-write oldest and then you would even get new contacts while disconnected at the expense of losing the oldest ones in the list. I'm not aware of any plans around auto-advert for companions, and not sure how likely that would be. Advert packets are some of the largest packets, and they are already making up a large proportion of traffic (and collisions) on the larger meshes so adding more to the mix would not be welcomed. Perhaps in future the discover-nodes feature could be expanded to cover companion nodes as well, with the option for companions who don't want to be found to turn off replying to those queries. |
|
Hello again. |
5 participants
This PR adds support for granular auto-add of specific contact types, and the option to overwrite the oldest non-favourite contact when the contact table is full.
Changes:
Added persisted autoadd_config bitmask for contact type auto-add control and overwrite oldest when full.
Bootstrap the RTC from the latest contact.lastmod timestamp to maintain monotonic time across reboots even when time hasn't been synced.
Added commands for getting and setting the auto-add configuration bitmask.
Added push codes to support keeping the client app in sync with contact overwrite and full-table conditions.
It's backwards compatible with older client apps, but the apps will require updates to support the new features. In the meantime you can toggle the new settings manually with a BLE serial client and later today I'll post the python script I've been using to change the settings while testing.
I'll mark this as draft until I can get some feedback from @liamcottle and @ripplebiz 🙏