Skip to content

Conversation

@lawik
Copy link
Contributor

@lawik lawik commented Aug 8, 2025

For the purpose of making it possible to watch live updates more information should refresh on connection changes. But it should be throttled in a responsible way.

We can even let fwup_progress events indicate that things should refresh if the pace is safe.

One big addition to making this safe is to determine which tabs are not being watched and if they are not watche we don't update them.

This improves on the 10 second timer updater.

@lawik lawik requested review from joshk and nshoes August 8, 2025 10:04
@lawik
Copy link
Contributor Author

lawik commented Aug 8, 2025

Okay, some tests regressed, gotta fix that.

@lawik lawik force-pushed the live-refresh-device-list branch from 495f962 to bee25b6 Compare September 1, 2025 13:08
@nshoes
Copy link
Contributor

nshoes commented Sep 2, 2025

I looked into the failing tests (since I wrote them hehe) and I uncovered a race condition, but one that is unique to the test env and doesn't reflect a real-world issue.

However, it does uncover that we're now doubling the work on a connection:change or connection:status with:

  • LV receives connection:* message
  • safe_refresh is called
  • assign_display_devices is called, starting an async update
  • update_device_statuses is called, updating the status
  • assign_display_devices async work is finished, updating the status

Or:

  • Tracker tells us that a device connection changes
  • We ask the Tracker the conneciton status of a device

The tests didn't mock Tracker, so it would default to offline, failing the tests, which would check if the green dot is visible when a device comes online.

I implemented a fix, but I don't love it. I think it's fine as a temporary; @joshk you've talked about getting rid of Tracker or changing the implementation, but I'm unsure what that entails or if it will happen soon.

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.

3 participants