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

Setting up RXV 2060 fails on ZONE_4 #25

Open
rawdlite opened this issue Oct 28, 2016 · 27 comments
Open

Setting up RXV 2060 fails on ZONE_4 #25

rawdlite opened this issue Oct 28, 2016 · 27 comments
Assignees
Labels
Milestone

Comments

@rawdlite
Copy link

The Yamaha RX V2060 has a Zone 4 without Mute or Volume (it is HDMI only)
This leads to the request for the mute state to return res.content as b''.
and then:
File "/home/hass/.homeassistant/deps/rxv/rxv.py", line 91, in _request
response = ET.XML(res.content)
File "/usr/lib/python3.4/xml/etree/ElementTree.py", line 1326, in XML
return parser.close()
File "", line None

=> Error while setting up platform yamaha

I think it might be best to allow zones to be ignored.
Furthermore exceptions from calls to ET.XML should probably be catched.

@wuub wuub self-assigned this Oct 28, 2016
@wuub
Copy link
Owner

wuub commented Oct 28, 2016

@rawdlite thanks for the report. We'll try to fx it but we will probably need your direct help with testing it out on real hardware.
Have you reported this in Home Assistant as well?

Cc: @sdague

@rawdlite
Copy link
Author

Sure i will test any fixes.
Just started to look into HA yesterday. Still trying to find my way around in the codebase.
I have not reported this in Home Assistant (yet).

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

Oh, interesting...

When does this failure happen? During initial setup? Also, could you attach your desc.xml for the system? (Probably http://ip.of.receiver/YamahaRemoteControl/desc.xml)

We will probably actually need to properly introspect the desc.xml for features on all the zones, and would be useful to see what this one looks like.

@rawdlite
Copy link
Author

Happens during initial setup when update() is called.

desc.xml is now at https://gist.github.com/rawdlite/bd23a2fa108a4459e061d8bb0f034a8f

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

This is where it's going to be going wrong in HA, there just isn't the idea that a media player doesn't have these things - https://github.com/sdague/home-assistant/blob/c89592af7d09f50fe1006798a77a5f752ce5fb55/homeassistant/components/media_player/yamaha.py#L100-L101

That's going to get run at the end of init (and then also when changes happen later).

We may just want to hide zones that don't have these controls

@wuub
Copy link
Owner

wuub commented Oct 28, 2016

@sdague if we do capabilities detection right we should be able to modify https://github.com/sdague/home-assistant/blob/c89592af7d09f50fe1006798a77a5f752ce5fb55/homeassistant/components/media_player/yamaha.py#L155 to return correct bitset per zone.

@wuub wuub added the bug label Oct 28, 2016
@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

Yeh, there looks like there is nearly no controls on Zone_4 https://gist.github.com/rawdlite/bd23a2fa108a4459e061d8bb0f034a8f#file-rx-v-2060-desc-xml-L1375

vs. Zone_3 (for instance - https://gist.github.com/rawdlite/bd23a2fa108a4459e061d8bb0f034a8f#file-rx-v-2060-desc-xml-L1250)

There are probably 2 options here that make sense.

  1. don't bother supporting zones that don't have this basic function, just not return Zone_4 in this case.

  2. fully do capabilities, and modify the update to not try to set these things if the capability says not to. HA already understands these as capabilities.

@wuub wuub added this to the v0.3.1 milestone Oct 28, 2016
@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

@wuub heh, posts crossing mid flight with same ideas. :)

@rawdlite the capabilities fix is the right long term one, but it's more complicated, and is going to take a bit.

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

@wuub we're also going to have to modify receiver.basic_status, that assumes Mute / Volume properties as well.

@rawdlite
Copy link
Author

Yes, i can see that.

What about
CONF_ZONE_IGNORE = 'zone_ignore'
for starters?
This might be interesting for users that do not use all Zones.

@wuub
Copy link
Owner

wuub commented Oct 28, 2016

@rawdlite if you're happy with only the MAIN_ZONE you can drop Yamaha autodiscovery and specify host manually. No zone detection will be done then, and this bug should not trigger.

@rawdlite
Copy link
Author

rawdlite commented Oct 28, 2016

Actually i have:

- platform: yamaha
    name: 'Living Room Receiver'
    host: 192.168.1.11
    source_names:
      AV1: "PC"
      AV2: "BD"
      AV3: "Raspi"
      AUDIO1: "Mopidi"
      TUNER: "Radio"

yet auto discovery is done.
Anyway i do need Zone_2 and Zone_3.

i probably hack at
https://github.com/sdague/home-assistant/blob/c89592af7d09f50fe1006798a77a5f752ce5fb55/homeassistant/components/media_player/yamaha.py#L69
to exclude my Zone_4

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

I intergrated generic discovery into Home Assistant recently (though I didn't think it was in a release yet), so you'd also have to turn off the discovery component.

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

@rawdlite yeh, that would work. I can put together and ignore zones patch for HA now

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

@rawdlite can you file a Home Assistant bug for me to work against here?

@rawdlite
Copy link
Author

sure.

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

Can you check out that this branch fixes things for you? - https://github.com/sdague/home-assistant/tree/yamaha_zone_ignore

If so, I'll PR that today.

@rawdlite
Copy link
Author

yep, setting it up now.

@wuub
Copy link
Owner

wuub commented Oct 28, 2016

FYI: I'm attending PyCon.CZ until late Sunday it's going to be difficult
for me to test stuff on real hardware until Monday :(

On Fri, 28 Oct 2016, 12:23 rawdlite, [email protected] wrote:

yep, setting it up now.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#25 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAj8OeWqO3pgkR4OkrTGFZgGlOam6bmlks5q4c0kgaJpZM4KjOgu
.

@rawdlite
Copy link
Author

No sure i did the setup right:

  • checked out branch
  • created new vitualenv
  • script/setup
  • added ignored zone to configuration.yaml

i get:

ERROR:homeassistant.components.media_player:Error while setting up platform yamaha
Traceback (most recent call last):
  File "/home/hass/home-assistant/homeassistant/helpers/entity_component.py", line 149, in _async_setup_platform
    entity_platform.add_entities, discovery_info
  File "/usr/lib/python3.4/asyncio/futures.py", line 388, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.4/asyncio/tasks.py", line 286, in _wakeup
    value = future.result()
  File "/usr/lib/python3.4/asyncio/futures.py", line 277, in result
    raise self._exception
  File "/usr/lib/python3.4/concurrent/futures/thread.py", line 54, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/home/hass/home-assistant/homeassistant/components/media_player/yamaha.py", line 74, in setup_platform
    YamahaDevice(name, receiver, source_ignore, source_names))
  File "/home/hass/home-assistant/homeassistant/helpers/entity_component.py", line 281, in add_entities
    ).result()
  File "/usr/lib/python3.4/concurrent/futures/_base.py", line 402, in result
    return self.__get_result()
  File "/usr/lib/python3.4/concurrent/futures/_base.py", line 354, in __get_result
    raise self._exception
  File "/usr/lib/python3.4/asyncio/tasks.py", line 237, in _step
    result = next(coro)
  File "/home/hass/home-assistant/homeassistant/helpers/entity_component.py", line 289, in async_add_entities
    tasks = [self._async_process_entity(entity) for entity in new_entities]
TypeError: 'YamahaDevice' object is not utterable

and

Exception in thread Thread-7:
Traceback (most recent call last):
  File "/usr/lib/python3.4/threading.py", line 920, in _bootstrap_inner
    self.run()
  File "/home/hass/.homeassistant/deps/netdisco/service.py", line 59, in run
    self._scan()
  File "/home/hass/.homeassistant/deps/netdisco/service.py", line 76, in _scan
    for service in self.discovery.get_info(disc):
  File "/home/hass/.homeassistant/deps/netdisco/discovery.py", line 79, in get_info
    return self.discoverables[dis].get_info()
  File "/home/hass/.homeassistant/deps/netdisco/discoverables/__init__.py", line 40, in get_info
    self.info_from_entry(entry) for entry in self.get_entries()))
  File "/home/hass/.homeassistant/deps/netdisco/discoverables/__init__.py", line 40, in <genexpr>
    self.info_from_entry(entry) for entry in self.get_entries()))
  File "/home/hass/.homeassistant/deps/netdisco/discoverables/yamaha.py", line 13, in info_from_entry
    yam['X_serviceList']['X_service']['X_controlURL'][1:])
TypeError: list indices must be integers, not str

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

Yeh, my bad, I'm working on an update now

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

Do a force pull on that branch again, it should be working now.

@rawdlite
Copy link
Author

Yes that looks good. Component is set up now.
Unfortunately i see errors when changing volume or source.
Will investigate further and report back later.

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

It is possible that the interface for this receiver changed enough that the code we have doesn't work with it. Probably worth logging the request / response out of the actually POST calls through requests.

Power and Volume control look the same between your desc.xml and the one I've got for my receiver. So volume is really the one that would be worth figuring out first.

@rawdlite
Copy link
Author

i don't know whether this is related or not, but i checked against your dev branch and i only see the following error with the yamaha_zone_ignore branch.
When changing volume for the first time i get:

Exception in thread Thread-7:
Traceback (most recent call last):
File "/usr/lib/python3.4/threading.py", line 920, in _bootstrap_inner
self.run()
File "/home/hass/.homeassistant/deps/netdisco/service.py", line 59, in run
self._scan()
File "/home/hass/.homeassistant/deps/netdisco/service.py", line 76, in _scan
for service in self.discovery.get_info(disc):
File "/home/hass/.homeassistant/deps/netdisco/discovery.py", line 79, in get_info
return self.discoverables[dis].get_info()
File "/home/hass/.homeassistant/deps/netdisco/discoverables/init.py", line 40, in get_info
self.info_from_entry(entry) for entry in self.get_entries()))
File "/home/hass/.homeassistant/deps/netdisco/discoverables/init.py", line 40, in
self.info_from_entry(entry) for entry in self.get_entries()))
File "/home/hass/.homeassistant/deps/netdisco/discoverables/yamaha.py", line 13, in info_from_entry
yam['X_serviceList']['X_service']['X_controlURL'][1:])
TypeError: list indices must be integers, not str

After that further changes of volume and source work as expected. I could not replicate the error i have seen earlier.

@sdague
Copy link
Contributor

sdague commented Oct 28, 2016

That should not actually be related. That just means the upnp discovery isn't working for your yamaha unit. That's probably only happening cooincidentally in time, because the upnp discovery takes about 15 - 20 seconds from boot to get results back and feed them to Home Assistant.

It might be an interesting thing to fix, but given that you specified the unit fully, it's not actually important here. If discovery was working, you'd actually see 2 versions of the stereo.

@rawdlite
Copy link
Author

OK, thanks for the quick fix then.

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

No branches or pull requests

3 participants