Skip to content

Comments

Add contact favorites filter with companion flags bit0 persistence#173

Open
Specter242 wants to merge 4 commits intozjs81:mainfrom
Specter242:feature/contact-favorites-filter
Open

Add contact favorites filter with companion flags bit0 persistence#173
Specter242 wants to merge 4 commits intozjs81:mainfrom
Specter242:feature/contact-favorites-filter

Conversation

@Specter242
Copy link
Contributor

@Specter242 Specter242 commented Feb 14, 2026

Summary

  • Adds a Favorites-only filter in contacts UI.
  • Uses companion contact flags bit0 as the source of truth for favorites (read + write), instead of local app-only storage.
  • Preserves contact flags when sending CMD_ADD_UPDATE_CONTACT path updates so favorites are not cleared by path changes.
  • Addresses reviewer feedback about cross-client persistence by removing the app-only favorites storage path.

Context

Test plan

  • dart format on modified Dart files
  • flutter gen-l10n
  • flutter analyze
  • Build/install on Android (flutter build apk --debug + adb install -r)
  • iOS test (not run: iOS device/environment unavailable)

Specter242 and others added 2 commits February 14, 2026 11:28
Introduce app-side room auto-sync orchestration with per-room enablement, sync status indicators, and configurable timing controls so users can keep only selected room servers up to date.

Co-authored-by: Cursor <cursoragent@cursor.com>
This adds a saved favorite flag per contact, exposes add/remove favorite in contact actions, and adds a Favorites filter in the contacts list to resolve issues zjs81#40 and zjs81#166.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ericszimmermann
Copy link
Contributor

ericszimmermann commented Feb 14, 2026

I can not see where the link to the contacts flag from the companion is made.
For me it seems it is only locally saved in a separate list. But I may be wrong, I haven't programmed in Flutter before.
To solve the Issue #166 it should manage the lowest bit (bit0) in the flags that are comming from the companion.
But I thank you for taking on these changes.
Sincerely Eric

Copy link
Collaborator

@446564 446564 left a comment

Choose a reason for hiding this comment

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

This does not appear to be using the flags as mentioned in #166

Or did I just miss that? I don't think we should be creating new patterns that don't persist with the radio across client apps.

@446564 446564 marked this pull request as draft February 14, 2026 18:05
This reads and writes favorite state via contact flags bit0, preserves flags when updating paths, and removes the app-only favorites storage pattern so favorite status stays consistent across clients.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Specter242 Specter242 changed the title Add contact favorites persistence and favorites filter Add contact favorites filter with companion flags bit0 persistence Feb 14, 2026
@Specter242
Copy link
Contributor Author

Thanks for the detailed review, Eric — great catch.

You were correct: the earlier version only used app-local favorite state. This has now been replaced.

What is now implemented in this PR:

  • Favorites are read from companion contact flags bit0.
  • Favorite toggles write back via CMD_ADD_UPDATE_CONTACT using updated flags.
  • Path updates now preserve existing flags, so favorite state is not accidentally cleared.
  • The app-only favorites cache/storage path was removed.

So the current PR behavior matches #166 expectations for cross-client consistency.

If you can, please retest on the latest commit (bc08298) and let me know if you still see any mismatch.

@Specter242
Copy link
Contributor Author

Quick follow-up after feedback: this PR now uses companion flags bit0 for favorites end-to-end (latest commit: bc08298).\n\nKey updates in code:\n- Parse/store contact flags from contact frames (, ).\n- Toggle favorite by writing with updated flags ().\n- Preserve flags during path updates so favorites are not lost on path changes.\n- Removed app-only favorite cache path.\n\nIf this looks good, please re-review against commit .

@ericszimmermann
Copy link
Contributor

Unfortunately I can not build and test at the moment, because I shot my build-system through loosing my environment variables and other links.
Therefore I could only check the files manually, but the code seems correct from what I could follow through the files.
I can see where you get the flag from the companion and where you set them.
Sincerely Eric

@Specter242 Specter242 marked this pull request as ready for review February 15, 2026 23:14
Copy link
Owner

@zjs81 zjs81 left a comment

Choose a reason for hiding this comment

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

@codex review

@zjs81
Copy link
Owner

zjs81 commented Feb 16, 2026

I reviewed this PR with the companion protocol behavior in mind, and these are my findings:

  1. [P1] Room sync does not restart when I re-enable it while already connected.
  • lib/services/room_sync_service.dart:121
  • lib/screens/app_settings_screen.dart:412
  1. [P1] Sync is marked successful on any single in-flight message frame, which can report a false "synced" state before queue completion.
  • lib/services/room_sync_service.dart:233
  • lib/services/room_sync_service.dart:287
  1. [P2] Toggling favorite sends full CMD_ADD_UPDATE_CONTACT (including path bytes/len), so favorite changes can also rewrite contact path state on-device.
  • lib/connector/meshcore_connector.dart:1125
  • companion reference: examples/companion_radio/MyMesh.cpp:163
  1. [P2] New room sync labels/settings are hardcoded English instead of localized.
  • lib/services/room_sync_service.dart:328
  • lib/screens/app_settings_screen.dart:401
  • lib/screens/contacts_screen.dart:505

Protocol check:

  • Favorite bit handling itself looks aligned (flags bit0).

@Specter242
Copy link
Contributor Author

Addressed all 4 findings from the latest review on this PR head ():

  1. Room sync now restarts when re-enabled while already connected.
  2. Sync success is now marked only on (queue completion), not on any in-flight message frame.
  3. Favorite toggle now refreshes contact snapshot from device before writing flags, so stale local path data does not get rewritten unintentionally.
  4. New room sync settings/status/labels are localized (removed hardcoded English in these paths).

I also ran Resolving dependencies...
Downloading packages...
characters 1.4.0 (1.4.1 available)
dbus 0.7.11 (0.7.12 available)
ffi 2.1.5 (2.2.0 available)
flutter_blue_plus 2.1.0 (2.1.1 available)
flutter_blue_plus_darwin 8.1.0 (8.1.1 available)
flutter_blue_plus_winrt 0.0.16 (0.0.18 available)
flutter_foreground_task 6.5.0 (9.2.0 available)
flutter_launcher_icons 0.13.1 (0.14.4 available)
flutter_lints 5.0.0 (6.0.0 available)
flutter_local_notifications 18.0.1 (20.1.0 available)
flutter_local_notifications_linux 5.0.0 (7.0.0 available)
flutter_local_notifications_platform_interface 8.0.0 (10.0.0 available)
flutter_map 7.0.2 (8.2.2 available)
hooks 1.0.0 (1.0.1 available)
lints 5.1.1 (6.1.0 available)
matcher 0.12.17 (0.12.18 available)
material_color_utilities 0.11.1 (0.13.0 available)
meta 1.17.0 (1.18.1 available)
mgrs_dart 2.0.0 (3.0.0 available)
mobile_scanner 6.0.11 (7.1.4 available)
objective_c 9.2.5 (9.3.0 available)
package_info_plus 8.3.1 (9.0.0 available)
petitparser 7.0.1 (7.0.2 available)
pointycastle 3.9.1 (4.0.0 available)
proj4dart 2.1.0 (3.0.0 available)
source_span 1.10.1 (1.10.2 available)
test_api 0.7.7 (0.7.9 available)
timezone 0.10.1 (0.11.0 available)
unicode 0.3.1 (1.1.9 available)
url_launcher_ios 6.3.6 (6.4.1 available)
wakelock_plus 1.3.3 (1.4.0 available)
Got dependencies!
31 packages have newer versions incompatible with dependency constraints.
Try flutter pub outdated for more information.
Analyzing meshcore-open...

info • The import of 'dart:typed_data' is unnecessary because all of the used elements are also provided by the import of 'package:flutter/foundation.dart' • lib/screens/repeater_status_screen.dart:3:8 • unnecessary_import locally after the changes.

Please re-review when you have a moment.

@Specter242
Copy link
Contributor Author

Correction to prior note (formatting issue):

Addressed all 4 findings from the latest review on this PR head (05cc2a2):

  1. Room sync now restarts when re-enabled while already connected.
  2. Sync success is now marked only on RESP_CODE_NO_MORE_MESSAGES (queue completion), not on any in-flight message frame.
  3. Favorite toggle now refreshes contact snapshot from device before writing flags, so stale local path data does not get rewritten unintentionally.
  4. New room sync settings/status/labels are localized (removed hardcoded English in these paths).

flutter analyze was run locally after these changes.

Please re-review when you have a moment.

@Specter242
Copy link
Contributor Author

@zjs81 @446564 requesting re-review on the latest commit when available. All listed findings have been addressed in 05cc2a2.

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.

Loosing Favorite flag Contact list filter "Favorites"

4 participants