Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: Added option to use SmartThings to fetch device power state #679

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

petarhristov94
Copy link

Description

I've tried to implement a little something that hopefully increases the reliability of the power state reading, as I personally too experienced the random "on" states in HomeKit, although the device was actually off (see #676, #663, #608, ). What I did is I've added power state reading over the SmartThings API.

Power State over SmartThings API

A code block for reading the state of the device over the SmartThings API was added. The code integrates with the existing logic inside the getPower() method of the remote.js. Should an error happen it is gracefully caught and the default pinging and HTTP local API flow is executed. I've tried to make it as less as invasive as possible and went against adding it inside base.js, as to avoid possible circular references. Also, the smart things logic is kept separate from the other ones, so the remotes.js felt like the best fit.

Under the SmartThings API section I've added the opt-in feature to use SmartThings API for power state readings to turn it on.
image

Furthermore, an API call is sent out once every 5 seconds. This seems to be quite fine for the endpoint's rate limiter (350 requests per minute allowed). The endpoint for the GET request is under https://api.smartthings.com/v1/devices/:deviceId/components/main/capabilities/switch/status. This information is also found under the general status endpoint (https://api.smartthings.com/v1/devices/:deviceId/status), but I've narrowed it down a bit. The response looks like so:

{
  "switch": {
    "value": "on", // either "on" or "off"
    "timestamp": "2024-04-27T20:43:46.563Z"
  }
}

Quality of Life Improvements

I've fixed few spelling errors and extended/improved the javascript documentation above some of the methods. Generally I tried to keep the same coding style as established in the project. Also, the additional debug logging you'll see I've added for myself to understand the inner workings better. I'd leave it up to you to decide if you'd like to keep it or not. I personally see no harm in keeping it.

Final thoughts

I do really like the approach you've taken first pinging the device's IP and then only if you it's reachable, you call the local device API (port 8001). Could not really understand why the random "on" state would appear in HomeKit, although it happened to me too, but never reproduced it successfully. I hope at least this change would help resolve the issues above.

@petarhristov94 petarhristov94 changed the title Added option to use SmartThings to fetch device power state Update: Added option to use SmartThings to fetch device power state Apr 27, 2024
@tavicu
Copy link
Owner

tavicu commented Apr 28, 2024

Hi Petar,

Unfortunately I cannot merge this PR because of multiple reasons.

The main ones are that the plugin is currently rewrite from scratch and the power state check is changed to another method I have found that should be much more reliable.

And the second is that the SmartThings API should be treated as an addon to the plugin. Because the basic functionality like detecting the power state and changing it should be locally and not remote. That means it should work without relying on internet.

I do appreciate your contribution and I guarantee you the new version will worth the wait :P
If you do wish to continue helping, I can ping you once I will release the beta version.

Could not really understand why the random "on" state would appear in HomeKit, although it happened to me too, but never reproduced it successfully. I hope at least this change would help resolve the issues above.

I have not encountered this problem either. But my guess is because some TVs respond slow to the http request and it hits the timeout limit which is 500ms (https://github.com/tavicu/homebridge-samsung-tizen/blob/master/lib/methods/base.js#L106). And if that happens then it fallbacks to the false (off) state (https://github.com/tavicu/homebridge-samsung-tizen/blob/master/lib/methods/base.js#L62).

@JasonGoldenDDT
Copy link

Thank you both for your work on this. It really limits the usefulness of this plugin.

I'm not a developer on Homebridge, but I use a few plugins and it would be great to get this one working.

I question why the device state needs to be queried at all, I notice the TV status updates in HomeKit immediately after I use my physical remote to turn it on or off.

Can this plug in just rely on this update from the device?

I have my iPad open near by when watching tv and I notice the status cycle every 3 seconds, from on to off and back on again.

I use the remote primarily for turning the TV on, I do have a scene that turns the tv on and selects a different HDMI port. (Game Mode)

LMK how I can help to progress this fix.

@BK101X
Copy link

BK101X commented Jun 23, 2024

Hi tavicu,
I think I have the issue with the http timeout you describe above. Would it be a problem to increase the timeout value or will this lead to new problems?
Do you have an alternative idea how to deal with the issue? I use the tv to switch light on and off based on power status.
I would like to get this more stable. At the moment light turns off and a randomly.
Thanks in advance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants