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

The probe if online seems too slow for Siri #50

Closed
DanielWeeber opened this issue Jul 17, 2018 · 33 comments
Closed

The probe if online seems too slow for Siri #50

DanielWeeber opened this issue Jul 17, 2018 · 33 comments

Comments

@DanielWeeber
Copy link

DanielWeeber commented Jul 17, 2018

If I use Siri with HomePod to turn on the TV the response seems too slow for Siri to recognize. It says, that "some devices are not responding". After a few more seconds the "turning wheel" in HomeKit is gone and the Switch shows that the TV is turned on.
Maybe the check for the availability of port 3000 is too slow?

    {
        "accessory": "webostv",
        "name": "LG-TV",
        "ip": "<ip>",
        "mac": "<mac>,
        "keyFile": "./lgtvKeyFile",
        "pollingEnabled": true,
        "volumeControl": false,
        "channelControl": false
    }
@merdok
Copy link
Owner

merdok commented Jul 17, 2018

Unfortunately it takes some time until the TV is available in the network and responds as online, when it takes longer then this is the response which you will get, but the TV will should anyway turn on.
Do you use WiFi or LAN to connect your TV to the network?

@DanielWeeber
Copy link
Author

WiFi, but perfect reception.

Maybe the setState should fake the status of the fake Switch and at the next interval the normal function checks the real state

@DanielWeeber
Copy link
Author

And: the „boot“ of the TV is very fast, I have a pretty new OLED one ;)

@merdok
Copy link
Owner

merdok commented Jul 17, 2018

I think this could be due to WiFi. It doesn't matter how fast the TV boots, the WiFi needs some time authenticate and connect to the network. Maybe with a LAN connection it would be faster. Any chance for you to try it out?

@DanielWeeber
Copy link
Author

DanielWeeber commented Jul 18, 2018

It works with LAN.

I changed back to WiFi.
I implemented a "fake on" now. Works fine with WiFi/LAN. Siri does not say "devices do not respond" anymore.

While I was testing the Ping was not responding anymore after the TV turned off (to be expected), but after a few tries the Ping stayed stable even though the TV was offline.
I suspect the OLED refreshing part of my LG for this. The ping was not responding anymore this morning (after the TV was off for a while)

This confuses webosTvAccessory.prototype.setState, because it does check if the module is connected or not to see if the TV is on or not - not the actual status of the TV. I know that it is unable to check anything via the API, while Layer3 is not working. I removed the "if (!this.connected)" from the "Turn On" part - the WoL request can be send if the ping is reachable and the module connected.
I also removed the "if (this.connected)" part from the "Turn Off" part and added the function to connect before the Turn Off. Worked way better than.

Should I create a PR for this?
//edit: done. #52

@merdok
Copy link
Owner

merdok commented Jul 18, 2018

Ok, so i think #51 has something to do with it. I have an idea how to implement this properly.
It would be good for me to be able to reproduce this issue, could you tell how can i reproduce this with a high success rate?

@DanielWeeber
Copy link
Author

DanielWeeber commented Jul 18, 2018

For me its just plain simple. Just say "Hey Siri Turn off LG TV in living room or "Hey Siri Turn on LG TV in living room ".
Fails 8/10 times while turning off, 10/10 while turning on when connected only via WiFi

@DanielWeeber
Copy link
Author

This is a very valid point from moonchen:
#51 (comment)

We should check that also. If you share your idea I could have a look also! :)

@merdok
Copy link
Owner

merdok commented Jul 18, 2018

I will try it out today, if i can reproduce it then i will attempt a fix.
The point is like in #51 discovered the callback seems to be wrong. I would need to adjust them.

@merdok
Copy link
Owner

merdok commented Jul 18, 2018

@moonchen
@DanielWeeber

Ok, so there is definitely the issue with the power, so if i ask Siri to turn off the TV and it is already turned off it should report with success, that makes sense.
On the other way when the TV is off and i ask Siri to mute my TV i think the response should be "device is not responding" since the TV is really off and not responding. I think this is the correct behavior.
Thoughts?

@DanielWeeber
Copy link
Author

From a logical standpoint this is absolutely correct.

From a HomeKit standpoint I disagree kindly - @moonchen explained that perfectly. If it’s used in a HomeKit scene the scene will not complete successfully and Siri will report an error. I agree that the volumeService has no point in being used in a scene, but why should it throw an error in any case. Best would be if Siri reports something like the scene is completed successfully and if you mute a turned off TV alone it should report something like „this does not need to be executed“ or whatever - but this is not possible.

@merdok
Copy link
Owner

merdok commented Jul 18, 2018

I just tried it with my Hue lights, as soon as i cut the power of my lamps Siri will also responds with "device is not responding", so if anyone has such lamp in a scene it will also throw an error.
I can agree that the power of the TV should not throw an error, but i am not sure about the volume service....

@DanielWeeber
Copy link
Author

Yeah but in this case the device is actually not responding.
Compare it to the color setting: if you set a turned off lamp to „red“ it will respond that it worked, but not turn on the lamp.

@merdok
Copy link
Owner

merdok commented Jul 18, 2018

Actually when i set a turned off lamp to "red" it will turn the lamp on and set it to red.

@DanielWeeber
Copy link
Author

I actually tried it with my LightStrips before posting - not for me. Strange. But: no error in any case :)

@merdok
Copy link
Owner

merdok commented Jul 18, 2018

True...
I pushed a commit, hopefully that fixes the issue. Please let me know...

@DanielWeeber
Copy link
Author

Turn off twice: second time error
Turn on does not work at all and shows the button of the switch as „not responding“
Ping was responding while I tried to turn it on, maybe the OLED refresh again. As I am watching TV right now I will not wait for this to turn off. The is.connected check before sending the WoL request makes no sense!

@merdok
Copy link
Owner

merdok commented Jul 18, 2018

Please if you do the tests then make sure that the OLED refresh is not running because this is just delivers wrong results and unnecessary confusion...
Your OLED tv is only off when you actually hear a mechanical "click" sound coming from the TV.

@DanielWeeber
Copy link
Author

You cannot turn this off. This is not the pixel refresh which takes an hour or so.The TV will turn on at that point when you sent the WoL request.

Also I just turned the TV via your Module off and it showed up on again in HomeKit - even though the TV was off.

@moonchen
Copy link

As far as muting, there is an unfortunate side effect of the volume control being a light bulb.

If I tell Siri to “turn off lights”, it will try to mute the TV as well. If the TV is off at that time, I will get “device is not responding”.

I understand the developer argument for erroring out, but as a user I wouldn’t want the error message, since everything happened according to my wishes.

@moonchen
Copy link

I would go further to say that unmuting while the TV is off should succeed as well.

The only time I expect the “not responding” message is when the TV is actually disconnected from the network, power, is broken, or is misconfigured.

@merdok
Copy link
Owner

merdok commented Jul 19, 2018

@DanielWeeber Like i said before you test it make sure it is off, you need to wait about 10 minutes before testing it, all other tests are just misleading...

@moonchen That is true, i noticed that too that when i want to turn off all light it also tries to mute the TV, that is why there is a switch which allows you to disable the volume service.

This I am not sure about, for my understanding is the "device is not responding" should come up when the tv is actaully not responding, that includes also when the TV is in standby since any LG API doesn't work at that time thus the TV cannot respond. You will get the same error message with any other HomeKit device when it is unreachable and not responding.

If you are not at home and try to mute the TV but the TV is off you get a success message but actually the TV is not responding so you got a wrong feedback, and when you come home you will see that the TV is not on which is what you have not expected.

This is a double sided card and this needs to be carefully though off.
Anyway in my latest commit the mute will always responds as success, but i am not sure yet if this is correct...

@DanielWeeber
Copy link
Author

With another module (homebridge-marantz-volume) we came to the conclusion to change the LightBulb to a Service.Fan. We only had to change two lines for that. We also made it optional via the config file.@marcin: Same Problem this morning.
Why should I have to wait for 10 minutes for a module to not give me an error?

@merdok
Copy link
Owner

merdok commented Jul 19, 2018

Making this configurable is not a solution i think, i would rather keep the configuration options simple. At some point I and the users will lose track of all the configuration options if i add one for every simple thing...
Didn't iOS 11.4 add speaker support? It would make sense because of HomePod. Or maybe is it iOS 12 Beta?
I think at some point Apple announced that they will be adding a speaker to HomeKit...

Because the TV is still on in those 10 minutes, this is how OLEDs work. I will try to somehow fix the OLED problem but this is not part of this issue here.

@DanielWeeber
Copy link
Author

DanielWeeber commented Jul 19, 2018

But it's not a simple thing, it's a valid point to change. Also the default for everyone would not be changed. If you do not specify the configuration option, nothing will change.
Example: https://github.com/robertvorthman/homebridge-marantz-volume/pull/16/files

Yes, Speaker Support is added within HomeKit, but not within Home.app. If you add a Speaker it would be available in other HomeKit Apps or Eve, but not within the OG Home App.

Also merdok that was exactly my issue here.. Everything which I noticed from time to time for turning off/on my OLED via the module.

@merdok
Copy link
Owner

merdok commented Jul 19, 2018

I would rather prefer not to do the config option at this time. The mute should now always return success so there will not be any issues with scenes.

Can such speaker device be controlled via Siri?

The OLED issue is not easily fixed, i already explained it. It is because the TV still is physically on, the screen is just turned off when the small pixel refresh is running...
But i will investigate this as soon as this Siri issue is fixed.

My latest should fix the Siri response when TV is on...

@DanielWeeber
Copy link
Author

DanielWeeber commented Jul 19, 2018

Can such speaker device be controlled via Siri?

No, as per my knowledge.

@merdok
Copy link
Owner

merdok commented Jul 19, 2018

Isn't HomePod such a speaker device?

@DanielWeeber
Copy link
Author

DanielWeeber commented Jul 19, 2018

You cannot emulate any Airplay2 devices, yet.
And the Service.Speaker does not work with Home.app - I tried it with the HomeKit Accessory Simulator from XCode a few days ago.

//edit: Let me explain the Service.Speaker again, I forgot something: It is possible to use that, but only as part of a video-doorbell. Not as a standalone speaker.
Per Example: I can use my door webcam as intercom (hear and talk through the camera).

@merdok
Copy link
Owner

merdok commented Jul 19, 2018

That is odd...
Anyway, the Siri not responding should be fixed now for both power state and mute state.

@merdok
Copy link
Owner

merdok commented Jul 21, 2018

@moonchen did you test my latest commit? Is your issue resolved?

@moonchen
Copy link

Yes. Siri no longer says "device not responding" if the TV is off.

@merdok
Copy link
Owner

merdok commented Jul 21, 2018

Ok, good to know.
I'm closing this issue then.
@DanielWeeber for the OLED problems, do you want to open a new issue?

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

No branches or pull requests

3 participants