tavicu / homebridge-samsung-tizen

Homebridge plugin for Samsung TV's with Tizen OS
MIT License
639 stars 86 forks source link

Feature Request: enable "delay" to queue commands instead of ignoring them #503

Closed seidnerj closed 2 years ago

seidnerj commented 2 years ago

For example, when turning a TV on then immediately off, given one has set the proper delay, the power off command is ignored and you are presented with an error in the log saying "Powering on is already in progress". I think it would be good you could opt to have the command queued for the specified delay time, instead of simply ignored.

If this is done, you then get the desired effect, albeit delayed.

One potential issue with the above is that you could end up with a very long queue of ON-->OFF-->ON-->OFF etc. so there's need to be some kind of limit to the number of commands that can be sent in a given time frame.

A complementary feature might be to have a delay in the processing so that consecutive commands, within a given time period, that cancel each other out are not processed, depending on the current state.

For example, if the current state is OFF, and the commands send are ON-->OFF-->ON-->OFF-->ON, eliminate all occurrences of ON-->OFF since those result in the current state.

Another example - if the current state is ON however, and the commands send are OFF-->ON-->OFF-->ON-->OFF, eliminate all occurrences of "OFF-->ON".

Another way of looking at this is, given only two states, within a given time period, only consider the last state requested. If it is the same as the current one, ignore the entire sequence. If not, then ignoring everything but the last command.

Thanks in advance.

tavicu commented 2 years ago

Hi,

The plugin structure was build that way and changing it will take hours of coding for something that i don't think it makes sense at all.

Why would you play with the toggle on-off-on-off-on-... ?

Since 2018 when i released this plugin (2018, WOW!, don't even know when time passed) you are the first one to have a problem with this :)

seidnerj commented 2 years ago

Well, I assume you don't have kids (or have out of the ordinary disciplined ones) otherwise you wouldn't be so surprised at this kind of a scenario.

As for the logic, I think it actually makes perfect sense since right now the behavior of a remote control for example and the plugin is inconsistent. If I use the a remote and switch the something on then immediately off then it ends up with a state of "off", not "on", whereas using the plugin it actually stays "on".

That really doesn't make sense in my opinion, as the last requested state is what the user actually wants.

But hey, I'm not here to argue with you. I just suggested a feature.

tavicu commented 2 years ago

The problem with Samsung TVs is that there is no good API to control the state and get it! Everything we do to control the state locally are hacks.

There are multiple things related and I tried to get an accurate response working with what we have.

For example your TV responds to ping for ~15 seconds after you turned it OFF! Even if it's off, checking only the ping will see it at ON. Newer TVs also report the state on an API so after we check the ping we also make a request to that API and see if it's on standby. For older TVs we just force the OFF status for 15 seconds unless you turn it on again from the Home App.

Also after turning the TV On/Off it will not accept requests for around 2.5 seconds. That's the reason the delay it's there.

Also after turning it OFF in the first ~15 seconds you must use one method to turn it ON, after that you need to user another method.

And many other hacks.

So as you can see there ar multiple things checked just to report if the TV is ON/OFF as accurate as we can and changing the way it works will require days of coding and testing for something it's not worth that much.

From what i know iOS allows you to block some apps. Maybe you could do it on your kids devices.

seidnerj commented 2 years ago

I get all of that, its pretty annoying - I agree. But what I'm talking about doesn't have to do with the state at all. All I'm saying is that during that period of "black hole", instead of the plugin responding with something like "Powering on is already in progress", it will queue that command and issue it after that "black hole" period of time has passed.

That way, during this time, the state of the TV will not change, but it will change after the command has been issued and the TV will end up in the state the user requested. This simply mimics what a user would manually do right now, wait until he/she can issue a command again and then re-issue it.

Maybe I'm missing something but I don't understand how this is related to detecting the state of the TV?

tavicu commented 2 years ago

Maybe I'm missing something but I don't understand how this is related to detecting the state of the TV?

Because the plugin supports other features that will trigger after the state is changing. And from the plugin I don't know if you changed the state from the Main Accessory, a switch, an automation, and so on :)

As I said, it's not impossible to do it but it will require hours of working which will not make sense at the moment because there are other things to be done.

I understand your request and I think it's a fair one, I will have on my to-do list but with a low priority.

seidnerj commented 2 years ago

No worries at all, I have no expectations here :) thanks!

jesec commented 2 years ago

A rather undesirable side effect of the current throw error when ... is already in progress logic is that the HomeKit client marks the device as non-responsive. The state lingers on and only goes away after a period of time. When this happens, the HomeKit client refuses to do anything with the device at all, and the TV remote will be switched to blank. This will even affect devices in the same gateway.

It would make sense to avoid throwing errors, store the target state, queue the command and report the target state to the client.

If it is not ... already in progress, we dispatch the command immediately to attempt to switch to the target state. Otherwise, a timer with a certain delay is set (and will be reset if the target state is modified again). There is no chaining as we only store the target state, there will be only one timer and only one or two commands are eventually dispatched to the device.

I can take a look at this when I have time. But, of course, I am OK if someone beats me to it.

tavicu commented 2 years ago

Hi @jesec,

Make sense. I also work on some other improvements. I will take a look on this one too.

But returning the state will not work because HomeKit doesn't expect a state. It expects a response if the command was success or failed.

jesec commented 2 years ago

Hi @jesec,

Make sense. I also work on some other improvements. I will take a look on this one too.

But returning the state will not work because HomeKit doesn't expect a state. It expects a response if the command was success or failed.

With this, we should always return success, given that eventually, the command would be successful. Currently, because the PoweringOnlineError, TvAlreadyOnlineError, or TvAlreadyOfflineError was thrown, the response could be a failure, and the HomeKit client viewed that rather unfavorably.

tavicu commented 2 years ago

I understand, but it was a failure in the end :) It's not like the command was successful.

If you didn't know about this and you just start using the plugin and toggle the accessory then toggle again and see the state didn't changed you will think there is a problem with the plugin and you will open an issue.

Having an error status and message in the console will be easy to understand what the problem was to avoid doing it in the future :)

Also, I must check but I don't think it remains "failed" for ever. It remains only until the sync is done automatically (every 5 seconds by default)

jesec commented 2 years ago

I understand, but it was a failure in the end :) It's not like the command was successful.

If you didn't know about this and you just start using the plugin and toggle the accessory then toggle again and see the state didn't changed you will think there is a problem with the plugin and you will open an issue.

Having an error status and message in the console will be easy to understand what the problem was to avoid doing it in the future :)

Also, I must check but I don't think it remains "failed" for ever. It remains only until the sync is done automatically (every 5 seconds by default)

;) The user could just take a look at their TV physically to know that something went wrong. The delay could be very short (500ms or so), so the command to switch to the target state will be dispatched (if different than the actual state) shortly after the existing power on/off is finished. Eventually, the actual state (TV, physically) would be the same as the HomeKit accessory state, and any desync would not last for long.

Error in this code path makes the HomeKit client complain, and the UX is not good with that. Indeed, it does not remain "failed" forever, for the logic parts (that is, it is possible to dispatch other commands after a while). However, there appears to be a visual bug and the "No Response" / ❗could linger for a very long time.