samemory / homebridge-eufy-security

Work in progress
Apache License 2.0
97 stars 14 forks source link

Possible fix for T8420 camera #326

Closed fullphat closed 2 years ago

fullphat commented 2 years ago

Specifically, the HomeKit Alarm System created for the camera not responding to changes in the Eufy app

schliemann commented 2 years ago

I need some time to verify the change.

fullphat commented 2 years ago

I need some time to verify the change.

That's fine! This is my first foray into git PRs and branches, and my first foray into Javascript, node and HomeBridge plugin development so I may well have got many things wrong! 😊 Just let me know if you have any queries. 👍🏼

fullphat commented 2 years ago

For info, the T8420 does support the ability to arm, disarm, schedule and geo-fence and you can enable any of motion detection, push notifications and siren for the home or away modes:

5FD9852F-AA20-4AB6-A2F6-01D78D7935DF

So having it as an Alarm System is HomeKit is really helpful. One thing I have spotted as well is that when the plugin starts, there is a log message along the lines of "Device is station but type is 3 might imply errors".

lenoxys commented 2 years ago

So having it as an Alarm System is HomeKit is really helpful. One thing I have spotted as well is that when the plugin starts, there is a log message along the lines of "Device is station but type is 3 might imply errors".

This is normal behavior to have this log message. Camera act as a station but doesn't have all station capabilities. That's why you get the warning message.

Btw, did you have a look on my comment on your PR ?

https://github.com/samemory/homebridge-eufy-security/pull/326#pullrequestreview-849121206

fullphat commented 2 years ago

So having it as an Alarm System is HomeKit is really helpful. One thing I have spotted as well is that when the plugin starts, there is a log message along the lines of "Device is station but type is 3 might imply errors".

This is normal behavior to have this log message. Camera act as a station but doesn't have all station capabilities. That's why you get the warning message.

That's fine.

Btw, did you have a look on my comment on your PR ?

#326 (review)

Apologies - I can't find it?

schliemann commented 2 years ago

@fullphat I'll take a look at this PR when I have pushed the changes to snapshot to the beta.

lenoxys commented 2 years ago
fullphat commented 2 years ago
  • isAlarmStation

Could you not point to a specific model while naming this function?

  • It would rather inverse have isAlarmStation than isFloodlight and avoid creating the object
  • Escape it like this if (!isAlarmStation()){ this.log.debug('does not provide alarm capabilities') return; }

Yes, I can do - is isAlarmStation() already defined somewhere? If not, how do I test if something is an alarm station?

Don't forget that the floodlight camera does provide an alarm function; it's more that it's its own station.

fullphat commented 2 years ago
  • isAlarmStation

Could you not point to a specific model while naming this function?

  • It would rather inverse have isAlarmStation than isFloodlight and avoid creating the object
  • Escape it like this if (!isAlarmStation()){ this.log.debug('does not provide alarm capabilities') return; }

Yes, I can do - is isAlarmStation() already defined somewhere? If not, how do I test if something is an alarm station?

Don't forget that the floodlight camera does provide an alarm function; it's more that it's its own station.

github-actions[bot] commented 2 years ago

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

schliemann commented 2 years ago

@fullphat Has this been fixed by some of @karesake fixes?

fullphat commented 2 years ago

@fullphat Has this been fixed by some of @karesake fixes?

I think it might have been but I haven't been able to check it properly yet. Give me a couple of days to confirm?

Using the Home app to change alarm state

Off - works and reflected in Eufy app Home - works and reflected in Eufy app Away - works and reflected in Eufy app Night - Home app stuck on "Arming..." until I leave the app and return to it when it reverts back to whatever state it was previously at.

github-actions[bot] commented 2 years ago

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions[bot] commented 2 years ago

We do not accept PR on this repo. Please submit your issue on this repo : https://github.com/homebridge-eufy-security/plugin/pulls, thanks. If you have any questions, please feel free to contact us.