Skip to content

Add sitemap diagnostics logging#1151

Open
timbms wants to merge 34 commits intodevelopfrom
feature/sitemap-performance
Open

Add sitemap diagnostics logging#1151
timbms wants to merge 34 commits intodevelopfrom
feature/sitemap-performance

Conversation

@timbms
Copy link
Copy Markdown
Contributor

@timbms timbms commented Apr 18, 2026

Summary

  • Adds SitemapDiagnostics — a guarded os.log enum that logs sitemap update timing, changed-row counts by widget kind, and view render events
  • Adds sitemapDiagnosticsLogging toggle to ApplicationPreferences (with explicit CodingKeys for safe Codable round-trips) and surfaces it in Debug Settings at runtime
  • Instruments SitemapPageViewModel.updateUI with timing and before/after row diffing — no changes to update logic
  • Adds a share-sheet export button to LogsViewer so logs can be pulled off device

This is a diagnostic-only change intended to guide future sitemap performance work. No sitemap update logic is modified.

Test plan

  • Enable "Sitemap Diagnostics Logging" in Settings → Debug
  • Open a sitemap and trigger updates (long-poll, foreground return)
  • Verify SitemapDiagnostics entries appear in Console.app under category SitemapDiagnostics
  • Verify logs are exportable via the share button in Settings → Debug → Logs
  • Disable toggle and confirm no diagnostic log entries are emitted

timbms added 5 commits April 18, 2026 08:46
Adds temporary diagnostic infrastructure to observe sitemap update
behaviour without any changes to the update logic itself.

- SitemapDiagnostics: new enum with guarded os.log calls for update
  timing, changed-row counts by kind, and view renders
- ApplicationPreferences: sitemapDiagnosticsLogging flag with explicit
  CodingKeys so the key survives Codable round-trips
- DebugSettingsView / SettingsView: toggle to enable/disable at runtime
- SitemapPageViewModel.updateUI: timing + row-diff instrumentation after
  rebuildRowInputs(); no logic changes
- LogsViewer: share-sheet export button for captured logs

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
CommonUI does not declare SFSafeSymbols as a dependency, so using
Image(systemSymbol:) caused a linker failure in the test scheme.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Allows CommonUI to use type-safe symbol references instead of
raw string-based Image(systemName:) calls.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
openhab-bot and others added 24 commits April 18, 2026 08:34
Signed-off-by: openhab-bot <info@openhabfoundation.org>
Three files to drop into $OPENHAB_CONF:
- stress_test.items: counter, 4 segmented items (2 cycling, 2 fixed),
  fast numeric items, slow controls, linked sub-page children
- stress_test.sitemap: reproduces the segmented:4 / linked:1-4 pattern
  seen in user reports; fixed Seg3/Seg4 act as a comparison artefact probe
- stress_test.rules: 1Hz flood rule + 10s slow-toggle rule

The fixed-value Seg3/Seg4 rows are the key diagnostic: if diagnostics
report changedKinds=segmented:4 instead of segmented:2, the row
comparison is unstable for that widget type.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
- Guard rowInputs assignment in rebuildRowInputs: only publish when the
  computed inputs differ from the current value, avoiding a spurious
  @published objectWillChange fire on every long-poll response.
- Move trackWidgetUpdates after rebuildRowInputs and skip it entirely
  when inputsChanged=false: prevents widgetUpdateVersions from being
  bumped for all widgets on polls where nothing actually changed.

Together these eliminate the two main sources of unnecessary SwiftUI
re-evaluation on high-frequency sitemap pages where most long-poll
responses carry no visible state change.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: openhab-bot <info@openhabfoundation.org>
Instead of bumping widgetUpdateVersions for every widget on each
long-poll response, zip old and new rowInputs and only increment
the version for widgets whose row content actually changed.

This makes the widgetVersion signal semantically correct: .onChange
on interactive rows (Segmented, Selection, Slider) now fires only
when that specific widget received a server update, not on every
unrelated poll. Falls back to bumping all widgets when the row count
changes (structural update).

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: openhab-bot <info@openhabfoundation.org>
Add a leading-edge throttle (250ms window) to the long-poll update
path. The first update after a quiet period applies immediately; rapid
subsequent updates within the window are coalesced — only the latest
server state is applied when the window expires.

The polling loop itself is unaffected and continues at full speed.
Three state fields track the throttle: lastUIUpdateAt, pendingLongPollPage,
and longPollDebounceTask. All three are cleared in startPageHandling,
stopPageHandling, and deinit to prevent stale state from a previous
run carrying over into a fresh page-handling session.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Prepend a header block to the log output containing app version,
build number, OS version, and hardware model identifier (e.g.
iPhone15,3). Uses ProcessInfo and utsname — no UIKit dependency.

This makes shared log files self-contained for bug reports without
requiring the reporter to manually provide device/version details.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Replace the single mapMs field with three separate timings:
  reconcileMs — reconcileWidgets + currentPage assignment
  sliderMs    — clearSyncedSliderOverrides
  rebuildMs   — rebuildRowInputs (row input mapper)

This makes it possible to pinpoint which phase dominates the
main-thread cost before committing to an off-main solution.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: openhab-bot <info@openhabfoundation.org>
Row input construction now reuses inputs for widgets whose content has
not changed since the last update. On each long-poll update a render key
(WidgetRenderKey) is computed per snapshot inside the already-detached
Task and compared against the cached keys from the previous build. Only
rows where the key differs are re-mapped through SitemapRowInputMapper;
unchanged rows return their existing SitemapRowInput directly.

For a 120-widget sitemap where a single item changes, this reduces mapper
calls from 120 to 1. The reusedInputCount is now surfaced in diagnostics
so it can be validated from TestFlight logs.

Supporting changes:
- WidgetMappingSnapshot: extract to its own file with SitemapRowInputSnapshotBuilder
  and SnapshotRowInputBuildResult (carries renderKeys + reusedInputCount)
- WidgetRenderKey and related key types gain Sendable so they can cross
  actor boundaries into the detached Task
- OpenHABWidget.WidgetType gains Sendable (needed by WidgetRenderKey)
- SitemapRowInputBuilderTests rewired to test the snapshot-based builder

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: openhab-bot <info@openhabfoundation.org>
SitemapNavigationView's scenePhase observer only calls refreshOnForeground()
on the root page ViewModel. When a linked (Group sub-page) is open and the
app goes to background, its ViewModel has no mechanism to re-poll on
foreground return — updates only arrived on the next long-polling cycle.

Add a scenePhase observer in SitemapPageView that calls refreshOnForeground()
when the scene becomes active, gated on viewModel.isLinked so the root page
(handled by SitemapNavigationView) is not refreshed twice.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
…crolls

The reconcile step (reconcileWidgets / copyWidgetProperties) copied server
properties from new OpenHABWidget objects onto existing ones to preserve
object identity. This cost 3-7ms of synchronous main-thread work on every
long-poll update and was the last significant source of main-thread blocking
that could cause dropped scroll frames.

Object identity is no longer required:
- Row rendering is driven by SitemapRowInput value types, not widget objects
- sliderValueOverrides and commandStates are keyed by item name, not object
- rowWidgetIndex is rebuilt from the current widgets on every update
- WidgetRenderKey change detection handles the "skip unchanged rows" concern

updateUI Phase 1 now just injects sendCommand closures and assigns the new
page — sub-millisecond work. The reconcileMs diagnostic field remains but
will consistently read 0-1ms.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Replace the scenePhase-based approach with a direct UIApplication
notification observer in SitemapPageViewModel. The scenePhase environment
value is not reliably propagated into NavigationLink destinations across
all iOS versions, making the previous approach intermittent.

Linked page ViewModels now observe UIApplication.didBecomeActiveNotification
directly in startObservers(), guarded by isLinkedPage. The observer task is
cancelled in deinit alongside the other observer tasks. The root page
continues to be handled by SitemapNavigationView's scenePhase observer,
which is unaffected.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
isLinkedPage was set after startObservers() in the linked-page init, so
the `if isLinkedPage` guard in startObservers() always saw false and the
UIApplication.didBecomeActiveNotification observer was never registered.

Move isLinkedPage = true before startObservers().

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: openhab-bot <info@openhabfoundation.org>
…names in logs

Guard all O(N) diagnostic analysis (changedRowCount, changedRowKinds) and
per-phase timing behind isEnabled so the work never runs when diagnostics
logging is off. Remove the dead structureChanged computation left over from
the reconcile removal.

Show a plain spinner instead of a redacted skeleton on linked page first load,
since the page structure is unknown until the initial poll returns.

Mark icon names and URLs as privacy: .public in all icon failure logs so
tester builds surface the actual icon identifier rather than <private>.
Promote WatchIconView failure logging from .debug to .error so it is
captured in installed-app logs at all.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: openhab-bot <info@openhabfoundation.org>
When an item has no sitemap label the server returns only the formatted
value in brackets, e.g. "[Uitgeschakeld]". WidgetMappingSnapshot.displayState
fell back to the raw label string when labelText was empty, causing the
bracketed value to render as label text and producing the duplicate display
"[Uitgeschakeld] Uitgeschakeld".

Remove the fallback so an empty labelText is passed through as-is; the row
views already handle this case by aligning the value to the trailing edge.

Add regression tests in WidgetMappingSnapshotDisplayStateTests covering the
bracket-only, normal label+value, and empty label cases.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
logger.info() messages are held in memory only and require Console.app to
be actively streaming from the device — they are not persisted in installed
builds without a special logging profile. Change to logger.notice() (the
default level) which is always persisted and visible in Console.app.

Also flatten the multiline string to a single line to avoid any compiler
edge cases with OSLogMessage interpolation across line continuations.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
The original issue was the multiline OSLogMessage string literal, not the
log level — info level is fully captured in the app's log export. Revert
to info while keeping the single-line string fix that resolved the actual
logging failure.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
timbms and others added 5 commits April 26, 2026 11:40
Removes ~370 lines of duplicated computed properties between OpenHABWidget
and WidgetMappingSnapshot (renderingKind, displayState, labelText, labelValue,
mappingsOrItemOptions, readOnly, iconState, etc.) into a single protocol
extension in OpenHABCore.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
Signed-off-by: openhab-bot <info@openhabfoundation.org>
Integrates bot-committed version bump (3.2.31/238) with local develop merge.

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
…tation layer

- Delete ColorPickerViewController (replaced by SwiftUI ColorPickerRowView) and its orphaned storyboard scene
- Delete ColorPickerView (unused FlexColorPicker-based SwiftUI wrapper)
- Delete NoIconDisplayableCell, NotificationTableViewCell, PlayerView, ReusableView, Throttler, UITableView extensions (unused UIKit leftovers)
- Remove stateValueAsBool/Brightness/UIColor from OpenHABWidget (dead code)
- Move preferredRowHeight logic from OpenHABCore model layer to WidgetMappingSnapshot in the presentation layer

Signed-off-by: Tim Mueller-Seydlitz <timbms@gmail.com>
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