dcnielsen90 / python-bravia-tv

MIT License
13 stars 6 forks source link

Not working on 109.x when 3.5mm connected #8

Closed rolfikr1 closed 4 years ago

rolfikr1 commented 4 years ago

The integration is partially broken since 0.109. I have my amplifier connected to 3.5mm jack out. With 0.109.2 I got this in logs: JSON request error:{ "result": [ [ { "target": "headphone", "volume": 22, "mute": false, "maxVolume": 100, "minVolume": 0 } ] ], "id": 1 } Since 0.109.3 the error is gone but integration still doesn't work when jack is connected. The only thing that works is power on. But there is no state update to Home Assistant. When jack is unplugged, it immediately start to work. Tested on KD-65XF7596.

Knapoc commented 4 years ago

The Hass integration does not support outputs other then the default speaker afaik. This library recently added support to choose an output other then the speaker. It will default to speaker, when no output is specified.

JPM-git commented 4 years ago

I come across this topic because @bieniu told me me on another topic i had, so i do understand why my issue may be closed,

However I cannot understand how was this testing done when I see people having problems with it, "basic" problems as nothing working (as in my case, i can't even get the status of the device to make automation work).

My main question is: Will this be reverted till the lib and this integration can fully replace the old one or is this to stay no matter what?

dcnielsen90 commented 4 years ago

@JPM-git if they are "basic" problems please submit a PR with your fix. We don't have every TV in every configuration so it is very hard to catch all issues without others feeback. If a previous version worked for you -- rollback to that version and submit an issue for the release that did not work. Understand that this is free work for many of us and we have jobs outside of this. As @bieniu said please open a new issue if you want your stuff to be looked at.

bieniu commented 4 years ago

@JPM-git You can always send us your TV and we will fix any issue.

JPM-git commented 4 years ago

@JPM-git if they are "basic" problems please submit a PR with your fix. We don't have every TV in every configuration so it is very hard to catch all issues without others feeback. If a previous version worked for you -- rollback to that version and submit an issue for the release that did not work. Understand that this is free work for many of us and we have jobs outside of this. As @bieniu said please open a new issue if you want your stuff to be looked at.

You miss understood me, i give many credits to the several thousand that made home assistant what it is today, specially knowing that most work is done without the usual access to the logs or setup of the users.

However there are braking changes that can't be ignore. I've made custom component in the past when the samsung component was update to the newer version and i had an older one. Ok so old version where deprecated in place of the newer interfaces and with new and improve functionalities.

However i don't see this as an improvement to bring new functionalities that the "old version" did't accomplish. Also passing from having full control to not even status is not something you don't even informed on the documentation, so users can upgrade with a known limitation.

My my point of view, and only based on your replays this is a known issue, undisclosed on the documentation and no warning was given, accepted by all the intervening on the process.

Also since even bug report is not welcome on this integration please keep up the good work and thank you for your time

PS: @bieniu thank you for all the replays you gave me on "all the topics I spammed" in order to get some useful information PS2: @bieniu you don't even what a log or a test, you just want a new tv? ok....

dcnielsen90 commented 4 years ago

Any how -- @rolfikr1; I'll take credit for breaking this. get_volume_info should only return None if it times out (the TV is off/not reachable) and return an empty dict() if the rest command was successful despite not containing the desired output. With that considered, it returns None when Home Assistant polls for volume if the headphone jack is plugged in. The code I wrote in Home Assistant assumes the TV is off when None is returned and exits the update function (also updating the instance to off). I wrote it that way so we don't have to wait 10 seconds per subsequent call to time out if your TV is off. I'll see if we can get a fix in as well as the feature to control the headphone output via Home Assistant this week.

bieniu commented 4 years ago

@JPM-git Everyone makes mistakes. It is normal for major changes in integration and the library to cause compatibility issues. Where were you when beta 0.109 was available? I tested eight different Sony Bravia TV models with beta version. How much have you tested? You complain a lot but it took you half a day to properly open the issue with a description of what actually doesn't work.

PS. Do you know what sarcasm is?

PS2. EOT

dcnielsen90 commented 4 years ago

Want to get some testing in before I publish the next release. Figured I'd share it before publishing. This should fix all the regression that has been had (such as this issue): pip install git+https://github.com/dcnielsen90/python-bravia-tv@dev

I'll publish the next release at the end of the week barring no new issues are discovered. We can then look at making the change in Home Assistant to make the headphone output get/set work.

Also

PS. Do you know what sarcasm is?

sarcasm aside I'll take a TV over logs any day.

bieniu commented 4 years ago

I tested exactly the same change (in get_volume_info) two hours ago. Entity status was correct with the mini jack plug connected.

thomasb89 commented 4 years ago

Can confirm this fixed the issue for me. Thx!

dcnielsen90 commented 4 years ago

@thomasb89 happy to hear it is working for you. This just got merged to dev in HA so I'm going to go ahead and close this issue.