fix: gate Intercom on server-side hasCommand instead of microphone/speaker props#25
Open
retrography wants to merge 1 commit into
Open
fix: gate Intercom on server-side hasCommand instead of microphone/speaker props#25retrography wants to merge 1 commit into
retrography wants to merge 1 commit into
Conversation
…eaker props Some doorbells that support two-way talk (notably BATTERY_DOORBELL_2 / S220) do not expose `microphone` or `speaker` boolean properties on the eufy-security device, so the previous gate in `createDeviceManifest` skipped the Intercom interface and HomeKit / Scrypted hid the talk button. Replace the property-presence check with `api.hasCommand(DEVICE_COMMANDS.START_TALKBACK)`, which consults the authoritative DeviceCommands table in eufy-security-client. This keeps existing behavior for cameras that report the boolean properties while correctly enabling Intercom for devices that support talkback but omit those flags. Tests updated to cover the supported, unsupported, and S220-style "talkback-supported-but-no-property-flags" cases.
0d164ed to
823ce71
Compare
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.
Summary
Some doorbells that support two-way talk — notably Video Doorbell S220 (BATTERY_DOORBELL_2) — do not expose
microphoneorspeakerboolean properties on the underlying eufy-security device. The current gate increateDeviceManifest(device-utils.ts:288-293) requires both properties to be present, so for these models theIntercominterface is never added and HomeKit / Scrypted hide the talk button.Replace the property-presence check with a server-side
api.hasCommand("deviceStartTalkback")call, which consults the authoritativeDeviceCommandstable ineufy-security-client. This is the same source of truth the server uses when actually executing talkback, so the manifest now matches what the device will accept at runtime.Repro
device.interfacesin the Scrypted REPL does not includeIntercomgetPropertiesfor the S220 returns a payload that does not includemicrophoneorspeaker(confirmed against the upstream property table forBATTERY_DOORBELL_2), so theproperties.microphone !== undefined && properties.speaker !== undefinedgate evaluates false and Intercom is skipped — even though the upstreamDeviceCommandstable for this model does includeCommandName.DeviceStartTalkback.Fix
Note on the string:
api.hasCommandproxies toDevice.hasCommandupstream, which checks the value against theCommandNameenum — those values are camelCase ("deviceStartTalkback"), not the snake-case WS protocol command name ("device.start_talkback"fromDEVICE_COMMANDS.START_TALKBACK). It would be nice to export theCommandNameenum from@caplaz/eufy-security-clientso callers don't have to hard-code the string, but that's out of scope for this fix.Tests
Updated
tests/unit/device-utils.test.ts:deviceStartTalkbackmicrophone/speakerproperties are absent (the S220 case)deviceStartTalkback, even ifmicrophone+speakerproperties are presentAll 11 tests in
device-utils.test.tspass.Test plan
npm test -- device-utils.test.tspasses locallydevice.interfacesnow includesIntercomfor an S220, and the HomeKit talk button works