fix: defer sensor enablement until device data is received

[tomquist]

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

Summary by CodeRabbit

• Bug Fixes

  • Fixed an issue where local API controls could start disabled; availability now updates once required device data is received.
  • Improved behavior when device version is unknown to avoid premature disablement.
  • Discovery configurations are re-published upon first data receipt for a device path to reflect accurate enablement.

• Documentation

  • Updated the changelog with a “Fixed” entry and removed a duplicate header.

[coderabbitai]

Walkthrough

Implements tri-state enablement for advertised components, deferring decisions until device data is available. Updates data handling to return updated paths, adds MQTT client tracking to republish discovery configs on first data receipt per path, adjusts Venus device predicates, and updates CHANGELOG. Public APIs updated accordingly.

Changes

Cohort / File(s)	Summary
Docs CHANGELOG.md	Added “Fixed” note for Venus race condition; consolidated “Next” header formatting.
Data handling flow src/dataHandler.ts, src/index.ts	handleDeviceData now returns string[] of updated paths; caller iterates paths and invokes mqttClient.onDeviceDataReceived for each. Error path returns [].
MQTT republish trigger src/mqttClient.ts	Added onDeviceDataReceived(device, publishPath) and a private Set to track first data per device/path; triggers discovery republish once per path.
Tri-state enablement support src/deviceDefinition.ts, src/generateDiscoveryConfigs.ts	enabled predicate type relaxed to `boolean
Venus predicates src/device/venus.ts	Local API advertise calls: enabled now returns undefined when deviceVersion unknown; otherwise deviceVersion >= 153. Previously evaluated unknown as disabled.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Device
  participant IDX as Index
  participant DH as DataHandler
  participant MQ as MqttClient
  participant GD as Discovery Generator

  Dev->>IDX: incoming device data (message)
  IDX->>DH: handleDeviceData(device, message)
  DH-->>IDX: updatedPaths: string[]

  loop for each updatedPath
    IDX->>MQ: onDeviceDataReceived(device, updatedPath)
    alt first time for device+path
      MQ->>GD: republish discovery for device
      GD->>GD: evaluate enabled(state): boolean | undefined
      alt enabled === true
        GD-->>MQ: publish config
      else enabled === false
        GD-->>MQ: publish null (disable)
      else enabled === undefined
        GD-->>MQ: skip (defer)
      end
    else already seen
      MQ-->>IDX: no-op
    end
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

In wires and whispers, I twitch my ear,
Paths light up once data nears.
Enable? Disable? I wait—unsaid,
’Til packets hop to wake the thread.
Venus blinks, the race is won,
Configs republished—job well done.
Thump-thump! Another sprint begun.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title accurately describes the core change of deferring sensor enablement until device data is received to address the race condition and is both concise and focused on the primary fix without extraneous details.

✨ 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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/index.ts (1)

220-230: Wrap the switch clause body to fix the lint error

const updatedPaths = ... is declared directly in the case 'device' clause. Without wrapping the clause in braces, the declaration is scoped to the entire switch, which violates our noSwitchDeclarations rule (reported by Biome) and can break compilation. Please wrap the clause in {} before declaring updatedPaths.

-        switch (topicType) {
-          case 'device':
+        switch (topicType) {
+          case 'device': {
             if (topics) {
               mqttClient.publish(topics.availabilityTopic, 'online', { qos: 1, retain: true });
               mqttClient.resetTimeoutCounter(device.deviceId);
             }
 
             deviceManager.clearResponseTimeout(device);
             const updatedPaths = dataHandler.handleDeviceData(device, message.toString());
             // Re-publish discovery configs for each message path that received data for the first time
             updatedPaths.forEach(path => mqttClient.onDeviceDataReceived(device, path));
             break;
-
-          case 'control':
+          }
+          case 'control':

🧹 Nitpick comments (2)

src/mqttClient.ts (1)

216-235: LGTM! Clean implementation of first-data-receipt tracking.

The method correctly implements the deferred sensor enablement feature by:

• Building a composite key to track device/path combinations uniquely
• Only republishing discovery configs on the first data receipt per device/path
• Logging appropriately at debug level

Optional consideration: The devicePathsWithData Set grows unbounded over the lifetime of the process. While the memory impact should be minimal (just string keys), in a very long-running deployment with many devices and paths, you might consider:

• Adding a method to clear stale entries if devices are removed
• Or documenting that the Set is intentionally unbounded for this use case

This is not a blocker—just a maintenance consideration for future reference.

src/generateDiscoveryConfigs.ts (1)

78-89: LGTM! Tri-state evaluation correctly implements deferred enablement.

The logic correctly handles three distinct cases:

• undefined (line 80-82): Defers decision by skipping publication (continue)
• false (line 84-87): Explicitly disables by publishing null config
• true/truthy (implicit else): Enables by proceeding to normal config publication

Note the use of strict equality (=== false) on line 84, which means only literal false triggers explicit disable—other falsy values are treated as enabled. This appears intentional and correct.

Optional consideration: The deviceState parameter is typed as any (line 24), which means type safety is relaxed when calling field.enabled(deviceState) on line 79. While this may be acceptable given the dynamic nature of device states in this system, you might consider:

• Adding runtime type guards in the enabled functions themselves
• Or documenting that enabled functions should handle unexpected state shapes gracefully

This is not a blocking issue—just a type safety consideration for maintainability.

📜 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 bc2c6ea.

📒 Files selected for processing (7)

• CHANGELOG.md (1 hunks)
• 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)

🧰 Additional context used

🧬 Code graph analysis (2)

src/dataHandler.ts (2)

src/types.ts (1)

• Device (212-215)

src/parser.ts (1)

• parseMessage (20-61)

src/mqttClient.ts (1)

src/types.ts (1)

• Device (212-215)

🪛 Biome (2.1.2)

src/index.ts

[error] 227-227: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: build (linux/amd64)
• GitHub Check: build (linux/arm64)
• GitHub Check: build-addon (linux/arm64)
• GitHub Check: build-addon (linux/arm/v7)

🔇 Additional comments (2)

src/mqttClient.ts (1)

14-14: LGTM! Well-chosen data structure for tracking first-time data reception.

The Set<string> efficiently tracks which device/path combinations have received data, supporting the PR's objective to defer sensor enablement until device data arrives.

src/generateDiscoveryConfigs.ts (1)

16-16: LGTM! Tri-state enablement aligns with PR objectives.

Extending the return type to boolean | undefined enables the deferred sensor enablement feature:

• true: component is enabled
• false: component is explicitly disabled
• undefined: defer decision until more data is available

This change is backward compatible since enabled is optional and existing boolean returns still work.