-
-
Notifications
You must be signed in to change notification settings - Fork 75
fix: return a device status from getState(), not the device itself - #1404
Open
JanzenMark wants to merge 1 commit into
Open
fix: return a device status from getState(), not the device itself #1404JanzenMark wants to merge 1 commit into
JanzenMark wants to merge 1 commit into
Conversation
Every characteristic getter reads fields off getState(), for example:
const s = await this.getState()
return typeof s.temperature === 'number' ? s.temperature : 0
getState() returned whatever client.getDevice() handed back, which is a
node-switchbot device instance. That instance exposes id, name, deviceType and
mac; the readings live behind await device.getStatus(). So s.temperature was
always undefined and the getter returned 0.
createDevice() then replaced the method with a delegator that returned the
client device directly:
device.getState = async () => {
try {
const dev = await client.getDevice(opts.id)
if (dev) {
return dev
}
} catch (e) { /* ignore */ }
return originalGetState()
}
Being an instance property, that shadowed the class implementation, so even
fixing the class alone had no effect at runtime.
One getState() serves every device class in genericDevice.ts and none override
it, so this affects all readings on all devices. It fails silently: HomeKit is
handed a plausible 0, or 100 percent battery, rather than an error.
Ask the device for its status, and drop the override. A device that reports no
status falls back to the existing minimal info rather than the instance, since
the instance looks like a state whose every field is missing.
Verified on a Meter Pro (CO2). Before, every value pushed to HomeKit was a
default except one:
CurrentTemperature=23.7, CurrentRelativeHumidity=0, BatteryLevel=100,
CarbonDioxideLevel=0
After, each value is a measurement:
CurrentTemperature=23.7, CurrentRelativeHumidity=59, BatteryLevel=100,
CarbonDioxideLevel=403
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Fixes #1403
getState()returned the node-switchbot device instance rather than thatdevice's status, so every characteristic getter read a field that is not on it
and fell back to a default. HomeKit received a plausible
0, or 100% battery,with nothing logged.
What changed
src/devices/genericDevice.ts— when the object fromclient.getDevice()canreport status,
await device.getStatus()and return that. Objects that arealready a plain status, and
{ body }responses, behave as before. A devicethat reports no status falls back to the existing minimal
{ id, type }rather than the instance, since the instance looks like a state whose every
field happens to be missing.
src/deviceFactory.ts— drop thegetStateoverride. It set an instanceproperty that shadowed the class implementation, so fixing the class alone had
no effect at runtime. The class already prefers the client-backed lookup.
Tests
test/device/getstate-returns-status.spec.tscovers: a status is returnedrather than the instance; plain status objects and
{ body }responses stillpass through; a failing or empty
getStatus()yields minimal info instead of theinstance;
createDevice()no longer replaces the method; and a meter reports areal temperature through
createHAPAccessory().Full suite, lint, typecheck and build pass.
Verified on hardware
Meter Pro (CO2), values pushed to HomeKit before:
after:
Scope
Deliberately limited to the one defect. I found several adjacent issues while
tracking this down — including CO2 never being exposed for the Meter Pro (CO2),
polled values being fetched then discarded, and two
node-switchbotproblems —and I would rather offer those separately than bundle them here.