Skip to content

[backport/1.4] Improve frontend: Deduplicate system-information requests - #4050

Open
joaoantoniocardoso wants to merge 4 commits into
bluerobotics:1.4-devfrom
joaoantoniocardoso:backports/1.4/dedupe-system-information-requests
Open

[backport/1.4] Improve frontend: Deduplicate system-information requests#4050
joaoantoniocardoso wants to merge 4 commits into
bluerobotics:1.4-devfrom
joaoantoniocardoso:backports/1.4/dedupe-system-information-requests

Conversation

@joaoantoniocardoso

@joaoantoniocardoso joaoantoniocardoso commented Jul 27, 2026

Copy link
Copy Markdown
Member

This is a backport of #4027 into 1.4.


Supersedes #4034 (recreated from fork joaoantoniocardoso/BlueOS-docker instead of same-repo head).

Subscribe only to cpu/memory/disk/temperature FetchTypes that have active consumers, and slow the always-on network poll to 2s.
Cpu, Disk, and CpuPie register only the system-information endpoints they display.
…ocal timer

Drop the duplicate 2s cpu/memory/disk interval and reuse shared store subscriptions.
Keep tray warnings and core-temperature reads fed without always-on temperature polling in the store.
@github-actions

Copy link
Copy Markdown

Automated PR Review

0. Summary

  • Verdict: MINOR SUGGESTIONS ✏️

Backport of #4027 to 1.4-dev. Introduces a reference-counted subscription API on the system-information store so that the Cpu/Disk widgets, SystemCondition, and HealthTrayMenu no longer each spin their own setInterval to poll overlapping endpoints. All subscribed types are now fetched by a single OneMoreTime task at 2 s cadence, and the task is paused when the last subscriber unmounts. HealthTrayMenu also gains explicit subscriptions for temperature+disk, so it works even when the System page hasn't been visited. The diff is small, focused, and mechanically consistent with master.

1. Correctness & Implementation Bugs

  • 1.1 [minor] core/frontend/src/store/system-information.ts:275-281subscribeSystemInformation reaches into OneMoreTime private state (isPaused, isRunning, timeoutId) via as any to disambiguate "never started" / "paused" / "already scheduled". This works today but is fragile: renaming any of those private fields in one-more-time.ts will silently regress the subscribe logic (starts get skipped or duplicated) with no compile-time signal. Consider adding a small public accessor on OneMoreTime (e.g. isActive(): boolean, or making the start() method idempotent w.r.t. a pending timeoutId) so the store doesn't need as any.
  • 1.2 [minor] core/frontend/src/store/system-information.ts:79-81fetchSystemNetworkTask delay changed from 10002000 ms. This is a user-visible behavior change (network graph updates half as fast) that isn't mentioned in the PR title/description and isn't logically part of "deduplicate system-information requests". Either call it out in the description or split into its own commit.

4. Performance

  • 4.1 [minor] core/frontend/src/widgets/Disk.vue:30 and core/frontend/src/widgets/Cpu.vue:96 — the standalone poll intervals were 30 s (Disk) and 5 s (Cpu); after this change both widgets subscribe to the shared 2 s task. When only one of these widgets is mounted (e.g. a dashboard with just the Disk widget), disk endpoint request frequency increases 15× and CPU 2.5×. The deduplication win still stands when multiple consumers are mounted, but the isolated-widget case gets more chatty than before. Confirm this is intentional.

6. Code Quality & Style

  • 6.1 [minor] core/frontend/src/store/system-information.ts:268-282subscribeSystemInformation mixes two responsibilities (counter bookkeeping + task lifecycle) with an eslint-disable + as any escape hatch in the middle. Extracting the "make sure the task is active" tail into a private helper (ensureTaskRunning()) would isolate the any and make intent clearer.
  • 6.2 [nit] core/frontend/src/store/system-information.ts:44-49 — a one-line comment explaining why system_information_subscribers is a module-scope map (rather than class state) would help future readers, since the rest of the store keeps its data on the class. Same pattern as the pre-existing notifier above it, so mentioning "kept outside Vuex reactivity" would suffice.
  • 6.3 [nit] core/frontend/src/store/system-information.ts:290has_subscribers uses snake_case while the surrounding local (fetches) and task names are camelCase. Root config has camelcase: 'off', so ESLint won't flag it, but consistency with the neighboring code would be hasSubscribers.

8. Documentation

  • 8.1 [nit] The three new public @Actions (fetchSubscribedSystemInformation, subscribeSystemInformation, unsubscribeSystemInformation) are the store's new public API. A short comment on each describing the reference-counted lifecycle (subscribe = ref++, unsubscribe = ref--, task auto-starts/stops) would save the next reader a trip through OneMoreTime.

Generated by PR Review Bot. This is advisory, a human reviewer must still approve.

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.

1 participant