Skip to content

Conversation

@tomquist
Copy link
Owner

@tomquist tomquist commented Oct 3, 2025

This fixes a race-condition where we disabled sensors before we received the first device data.

Summary by CodeRabbit

  • New Features

    • Auto-republishes Home Assistant discovery for a device the first time data arrives per metric/path, ensuring components appear as soon as they become eligible.
    • Component enablement can defer until required device data is available, providing more accurate, data-driven discovery.
  • Bug Fixes

    • Improved handling when device version is missing: components that depend on version no longer incorrectly disable; they defer until version is known, avoiding premature hiding or showing.

This fixes a race-condition where we disabled sensors before we received the first device data.
@tomquist tomquist closed this Oct 3, 2025
@coderabbitai
Copy link

coderabbitai bot commented Oct 3, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Changed data handling to return updated message paths and propagate them to trigger per-path discovery republish on first data receipt. Enabled predicates can now return boolean or undefined. Discovery generation now defers, disables, or publishes based on enabled outcomes. Added MQTT client tracking for device-path data to republish discovery once per path.

Changes

Cohort / File(s) Summary of Changes
Data handling return paths
src/dataHandler.ts
DataHandler.handleDeviceData now returns string[] of updated publish paths; accumulates updatedPaths during processing; returns [] on errors; docblock/signature updated.
Index integration with updated paths
src/index.ts
Captures updatedPaths from handleDeviceData and calls mqttClient.onDeviceDataReceived(device, path) for each to trigger discovery republish.
MQTT client first-data tracking
src/mqttClient.ts
Added devicePathsWithData: Set<string>; new onDeviceDataReceived(device, publishPath) logs first receipt per device+path and triggers discovery republish.
HA enabled predicate can be undefined
src/deviceDefinition.ts
AdvertiseComponentFn updated: `enabled?: (state: T) => boolean
Discovery generation handling for enabled outcomes
src/generateDiscoveryConfigs.ts
HaAdvertisement.enabled now `boolean
Venus device enable checks
src/device/venus.ts
Enabled predicate changed from (state.deviceVersion ?? 0) >= 153 to (state.deviceVersion == null ? undefined : state.deviceVersion >= 153), yielding undefined when version missing.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Device
  participant Broker as MQTT Broker
  participant Index as App Index
  participant DH as DataHandler
  participant MC as MqttClient
  participant GC as Discovery Generator

  Device->>Broker: Publish device data message
  Broker->>Index: Message received
  Index->>DH: handleDeviceData(device, message)
  DH-->>Index: updatedPaths[string[]]
  loop For each path in updatedPaths
    Index->>MC: onDeviceDataReceived(device, path)
    alt First data for device+path
      MC->>GC: Republish discovery for device
      GC->>GC: Evaluate enabled(state)
      opt enabled === undefined
        GC-->>MC: Skip publishing this item
      end
      opt enabled === false
        GC-->>MC: Publish null config (disable)
      end
      opt enabled === true or no check
        GC-->>MC: Publish config
      end
    else Already seen
      MC-->>Index: No action
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I nibble bits where messages hop,
Paths sprout like clover—pop, pop, pop! 🌱
First taste of data? I thump with glee,
Republish maps for HA to see.
If maybe, nay, or yes defined—
I twitch my ears, with states aligned. 🐇✨

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-sensor-enablement

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8f78ad6 and f4b6084.

📒 Files selected for processing (6)
  • src/dataHandler.ts (1 hunks)
  • src/device/venus.ts (2 hunks)
  • src/deviceDefinition.ts (1 hunks)
  • src/generateDiscoveryConfigs.ts (2 hunks)
  • src/index.ts (1 hunks)
  • src/mqttClient.ts (2 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

2 participants