Skip to content

feat(peripheral-demo): add Sensor Data card to operator view#725

Open
nicomiguelino wants to merge 6 commits intofeat/peripheral-integration-pocfrom
ew-demo/additional-sensor-data-on-operator-view
Open

feat(peripheral-demo): add Sensor Data card to operator view#725
nicomiguelino wants to merge 6 commits intofeat/peripheral-integration-pocfrom
ew-demo/additional-sensor-data-on-operator-view

Conversation

@nicomiguelino
Copy link
Contributor

@nicomiguelino nicomiguelino commented Mar 9, 2026

Overview

  • Add humidity and airPressure fields to AppState (nullable, default null)
  • Add setHumidity and setAirPressure mutations to state
  • Parse humidity and air_pressure readings from peripheral WS messages
  • Add Sensor Data glass card to operator screen with temperature, humidity, air pressure
  • Show "No Data" fallback until first WS message arrives
  • Handle nullable temperature in public view temperature pill
  • Add Raw Peripheral State glass card to maintenance view with syntax-highlighted JSON

Screenshot(s)

image image

- Add humidity and airPressure fields to AppState (nullable, default null)
- Add setHumidity and setAirPressure mutations to state
- Parse humidity and air_pressure readings from peripheral WS messages
- Add Sensor Data glass card to operator screen with temperature, humidity, air pressure
- Show "No Data" fallback until first WS message arrives
- Handle nullable temperature in public view temperature pill

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 9, 2026

PR Reviewer Guide 🔍

(Review updated until commit c84b3ad)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Input Validation

The WS parsing casts sensor fields to number without runtime validation. Consider guarding against missing/undefined states, unexpected shapes, and non-finite values (e.g., strings/NaN), so UI/state don’t get polluted with invalid readings.

client.watchState((msg: PeripheralStateMessage) => {
  const readings = msg.request.edge_app_source_state.states

  const tempReading = readings.find((r) => 'ambient_temperature' in r)
  if (tempReading) {
    setTemperature(tempReading.ambient_temperature as number)
  }

  const humidityReading = readings.find((r) => 'humidity' in r)
  if (humidityReading) {
    setHumidity(humidityReading.humidity as number)
  }

  const pressureReading = readings.find((r) => 'air_pressure' in r)
  if (pressureReading) {
    setAirPressure(pressureReading.air_pressure as number)
  }
Nullability

temperature, humidity, and airPressure are now nullable with null defaults. Ensure all other code paths that read these fields handle null safely (avoid Math.round/arithmetic on null) and that any external callers of setTemperature/setHumidity/setAirPressure don’t pass invalid values.

export interface AppState {
  currentScreen: ScreenType
  timezone: string
  locale: string
  temperature: number | null
  humidity: number | null
  airPressure: number | null
}

const state: AppState = {
  currentScreen: 'public',
  timezone: 'UTC',
  locale: 'en',
  temperature: null,
  humidity: null,
  airPressure: null,
}
DOM Access

updateSensorData queries elements via getEl on every state change regardless of current screen. Confirm those elements always exist in the DOM (even when screens are hidden) and consider caching the element references to avoid repeated lookups and potential runtime errors if markup changes.

function updateSensorData(state: ReturnType<typeof getState>) {
  getEl('sensor-temperature').textContent =
    state.temperature !== null
      ? `${Math.round(state.temperature)}°C`
      : 'No Data'
  getEl('sensor-humidity').textContent =
    state.humidity !== null ? `${Math.round(state.humidity)}%` : 'No Data'
  getEl('sensor-air-pressure').textContent =
    state.airPressure !== null
      ? `${Math.round(state.airPressure)} hPa`
      : 'No Data'
}

function onStateChange(state: ReturnType<typeof getState>) {
  if (state.currentScreen === 'operator') {
    startUpdates()
  } else {
    stopUpdates()
  }
  updateSensorData(state)
}

export function initOperatorDashboard() {
  subscribe(onStateChange)
  const state = getState()
  if (state.currentScreen === 'operator') startUpdates()
  updateSensorData(state)
}

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

Persistent review updated to latest commit c84b3ad

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

PR Code Suggestions ✨

Latest suggestions up to c84b3ad
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Support clearing sensor state values

Allow sensor setters to accept null so the app can explicitly clear values when a
reading is missing or invalid. Without this, stale sensor data can linger
indefinitely in state and the operator view will show outdated readings.

edge-apps/peripheral-integration-demo/src/core/state.ts [57-65]

-export function setHumidity(value: number) {
+export function setHumidity(value: number | null) {
   state.humidity = value
   notify()
 }
 
-export function setAirPressure(value: number) {
+export function setAirPressure(value: number | null) {
   state.airPressure = value
   notify()
 }
Suggestion importance[1-10]: 7

__

Why: This aligns the setter APIs with the AppState fields being number | null and enables clearing invalid/missing readings so stale values don’t linger. The improved_code is a correct minimal change to the shown functions.

Medium
Possible issue
Avoid crashes when elements missing

Guard against missing DOM nodes before calling getEl(...) so this feature doesn't
crash if the operator markup is not present (e.g., different build/route/layout).
Early-return when any sensor element is absent.

edge-apps/peripheral-integration-demo/src/features/operator-dashboard.ts [95-106]

 function updateSensorData(state: ReturnType<typeof getState>) {
-  getEl('sensor-temperature').textContent =
-    state.temperature !== null
-      ? `${Math.round(state.temperature)}°C`
-      : 'No Data'
-  getEl('sensor-humidity').textContent =
+  const tempEl = document.getElementById('sensor-temperature')
+  const humidityEl = document.getElementById('sensor-humidity')
+  const pressureEl = document.getElementById('sensor-air-pressure')
+  if (!tempEl || !humidityEl || !pressureEl) return
+
+  tempEl.textContent =
+    state.temperature !== null ? `${Math.round(state.temperature)}°C` : 'No Data'
+  humidityEl.textContent =
     state.humidity !== null ? `${Math.round(state.humidity)}%` : 'No Data'
-  getEl('sensor-air-pressure').textContent =
-    state.airPressure !== null
-      ? `${Math.round(state.airPressure)} hPa`
-      : 'No Data'
+  pressureEl.textContent =
+    state.airPressure !== null ? `${Math.round(state.airPressure)} hPa` : 'No Data'
 }
Suggestion importance[1-10]: 5

__

Why: Adding null-checks prevents runtime crashes if the operator DOM elements are absent, which is a sensible defensive guard. However, it bypasses the existing getEl(...) helper and may mask markup regressions by silently returning.

Low

Previous suggestions

Suggestions up to commit c84b3ad
CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent stale or invalid readings

Always update all three sensor fields on each message and set them to null when a
reading is missing, otherwise stale values can persist indefinitely. Also
validate/coerce the incoming values to prevent propagating NaN into the UI when the
peripheral sends unexpected types.

edge-apps/peripheral-integration-demo/src/features/peripherals.ts [24-37]

 const tempReading = readings.find((r) => 'ambient_temperature' in r)
-if (tempReading) {
-  setTemperature(tempReading.ambient_temperature as number)
-}
+const nextTempRaw =
+  tempReading && typeof (tempReading as any).ambient_temperature !== 'undefined'
+    ? Number((tempReading as any).ambient_temperature)
+    : null
+setTemperature(nextTempRaw !== null && Number.isFinite(nextTempRaw) ? nextTempRaw : null)
 
 const humidityReading = readings.find((r) => 'humidity' in r)
-if (humidityReading) {
-  setHumidity(humidityReading.humidity as number)
-}
+const nextHumidityRaw =
+  humidityReading && typeof (humidityReading as any).humidity !== 'undefined'
+    ? Number((humidityReading as any).humidity)
+    : null
+setHumidity(
+  nextHumidityRaw !== null && Number.isFinite(nextHumidityRaw) ? nextHumidityRaw : null,
+)
 
 const pressureReading = readings.find((r) => 'air_pressure' in r)
-if (pressureReading) {
-  setAirPressure(pressureReading.air_pressure as number)
-}
+const nextPressureRaw =
+  pressureReading && typeof (pressureReading as any).air_pressure !== 'undefined'
+    ? Number((pressureReading as any).air_pressure)
+    : null
+setAirPressure(
+  nextPressureRaw !== null && Number.isFinite(nextPressureRaw) ? nextPressureRaw : null,
+)
Suggestion importance[1-10]: 8

__

Why: The current logic only updates fields when a reading exists, so missing readings can leave old values displayed indefinitely; setting missing readings to null fixes that. Coercing/validating numeric inputs prevents propagating NaN to the UI and matches the UI’s null handling.

Medium
Support clearing sensor values

Allow the setters to accept null so the UI can be cleared when a reading disappears
(e.g., sensor disconnect). Also avoid calling notify() when the value hasn’t changed
to prevent unnecessary DOM updates and re-renders.

edge-apps/peripheral-integration-demo/src/core/state.ts [52-65]

-export function setTemperature(value: number) {
+export function setTemperature(value: number | null) {
+  if (state.temperature === value) return
   state.temperature = value
   notify()
 }
 
-export function setHumidity(value: number) {
+export function setHumidity(value: number | null) {
+  if (state.humidity === value) return
   state.humidity = value
   notify()
 }
 
-export function setAirPressure(value: number) {
+export function setAirPressure(value: number | null) {
+  if (state.airPressure === value) return
   state.airPressure = value
   notify()
 }
Suggestion importance[1-10]: 7

__

Why: This aligns with the PR’s change to store sensor fields as number | null and prevents stale UI by allowing values to be cleared when readings disappear. Skipping notify() when the value is unchanged is a reasonable optimization with low risk.

Medium

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds additional environmental sensor readings (humidity + air pressure) to the peripheral integration demo’s state and UI, so the operator view can display a consolidated “Sensor Data” card with sensible null/empty fallbacks before the first WebSocket message arrives.

Changes:

  • Extended AppState with nullable humidity and airPressure (and made temperature nullable) plus new mutations to update these fields.
  • Parsed humidity and air_pressure from peripheral WS state messages and stored them in app state.
  • Added an Operator “Sensor Data” glass card and updated operator/public UI rendering to handle null sensor values (“No Data” / --).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
edge-apps/peripheral-integration-demo/src/features/peripherals.ts Parses additional sensor readings from WS state and updates app state.
edge-apps/peripheral-integration-demo/src/features/operator-dashboard.ts Renders sensor values (or “No Data”) into the operator dashboard UI.
edge-apps/peripheral-integration-demo/src/core/state.ts Adds nullable sensor fields and mutations for humidity/air pressure.
edge-apps/peripheral-integration-demo/src/core/screen.ts Updates public temperature pill to handle nullable temperature.
edge-apps/peripheral-integration-demo/index.html Adds the new “Sensor Data” operator card with temperature/humidity/air pressure fields.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nicomiguelino and others added 2 commits March 9, 2026 11:37
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Update setTemperature, setHumidity, setAirPressure to accept number | null
- Keeps state API consistent with the nullable AppState field types

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nicomiguelino nicomiguelino force-pushed the ew-demo/additional-sensor-data-on-operator-view branch from 649b011 to 6285ffa Compare March 9, 2026 18:45
…ify call

- Replace individual setTemperature/setHumidity/setAirPressure setters with a single setSensorReadings batch mutation
- Update peripherals.ts to use setSensorReadings, reducing notify() calls from 3 to 1 per WebSocket message

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nicomiguelino
Copy link
Contributor Author

@salmanfarisvp, please review this PR when you get the chance. This merges into #719's branch.

- Install highlight.js for JSON syntax highlighting
- Store last peripheral WebSocket message in module-level state
- Add Raw Peripheral State glass card to maintenance screen
- Render highlighted JSON via highlight.js (Monokai theme) on maintenance view load

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@nicomiguelino nicomiguelino requested a review from 514sid March 10, 2026 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants