Harden remaining wrong-service characteristic fallback in history logger (humidity) - #330
Open
7onnie wants to merge 1 commit into
Open
Conversation
…tory fallback (naofireblade#317) When no separate HumidityService exists, the history logger fell back to accessory.CurrentConditionsService.getCharacteristic(CurrentRelativeHumidity). HAP-NodeJS lazily creates and attaches a characteristic on getCharacteristic(), so in modes where the main service is a bare TemperatureSensor (e.g. eve with extraHumidity) this silently mounts CurrentRelativeHumidity onto a service that does not permit it, tripping Homebridge 2.0 / iOS strict HAP validation ("Accessory out of compliance"). Guard the fallback with testCharacteristic() so the value is only read when the characteristic is legitimately present (eve, eve2, default modes still log the real value), and fall back to 0 otherwise instead of polluting the service. Mirrors the existing :0 guards already used for pressure and lux.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Related to #317 ("accessory is out of compliance since homebridge 2.0").
The fakegato history logger in
index.jsreads characteristic values directly off a service. For the humidity value it falls back toCurrentConditionsService.getCharacteristic(Characteristic.CurrentRelativeHumidity)when no dedicatedHumidityServiceexists.Service.getCharacteristic(...)lazily creates and mounts the characteristic if it isn't already present — so whenCurrentRelativeHumidityis not on that service, this call silently adds a characteristic that doesn't belong there. Under Homebridge 2.0 / strict HAP, a service carrying a non-conformant characteristic causes the accessory to be flagged out of compliance.Relationship to #327
PR #327 already hardens the lux fallback in this same block (
testCharacteristic(...) ? getCharacteristic(...).value : 0) and bumps fakegato. It does not touch the humidity fallback, which still uses the baregetCharacteristiccall. This PR applies the identical, safe guard to the humidity line so the last remaining wrong-service auto-add is closed.Fix
testCharacteristic(name)is a side-effect-free existence check (HAP-NodeJSService.testCharacteristic), so it never mounts anything. When the characteristic genuinely exists the real value is logged; otherwise0is used, matching the existing pressure/lux: 0idiom.Trigger scope
The fallback branch is only reached when there is no
HumidityServiceandCurrentRelativeHumiditywas not mounted on the main service (e.g.Humidityplaced in thehiddenconfig). It is narrow, but it is a real, deterministic out-of-compliance trigger.Validation (no hardware required)
testCharacteristicconfirmed side-effect-free against HAP-NodeJSServicesource.node --checkpasses on the changed file (repo is plain CommonJS, no build/lint/test). One-line change; no secrets in the diff.