ol-iver / denonavr

Automation Library for Denon AVR receivers.
MIT License
176 stars 67 forks source link

Error connecting to AVR when Dynamic EQ is off #170

Closed MoshiBin closed 3 years ago

MoshiBin commented 3 years ago

AVR Model: AVR-X2400H denonavr version: latest master 053727bf1ccc09fe629ddf0ea64d84898615167e

>>> from denonavr import DenonAVR
>>> d = DenonAVR(avr_ip)
~/denonavr/denonavr/denonavr.py in __init__(self, host, name, show_all_inputs, timeout, add_zones)
    342 
    343         # Get initial setting of values
--> 344         self.update()
    345         # Create instances of additional zones if requested
    346         if self._zone == MAIN_ZONE and add_zones is not None:

~/denonavr/denonavr/denonavr.py in update(self)
    525 
    526         if self._receiver_type == AVR_X_2016.type:
--> 527             return bool(self._update_avr_2016())
    528         elif self._support_update_avr_2016:
    529             if self._update_avr_2016() is not True:

~/denonavr/denonavr/denonavr.py in _update_avr_2016(self, compatibiliy_check)
    642 
    643             # Update Audyssey
--> 644             executor.submit(self._audyssey.update())
    645 
    646             try:

~/denonavr/denonavr/audyssey.py in update(self)
    109                 self.multeq = MULTI_EQ_MAP.get(param.text)
    110             elif param.get("name") == "dynamiceq":
--> 111                 self.dynamiceq = bool(int(param.text))
    112             elif param.get("name") == "reflevoffset":
    113                 # Reference level offset can only be used with DynamicEQ

TypeError: int() argument must be a string, a bytes-like object or a number, not 'NoneType'

I added some debug logging for the parsed params in audyssey.py and saw that for the dynamiceq param, the value of param.text was None rather than a number.

MoshiBin commented 3 years ago

Manually calling the GetAudyssey endpoint, I see that it's returning the values as an attribute named control rather than text in the node:

<?xml version="1.0" encoding="utf-8" ?>
<rx>
  <cmd>
    <name>GetAudyssey</name>
    <list>
      <param name="reflevoffset" control="0"></param>
      <param name="multeq" control="0"></param>
      <param name="dynamiceq" control="0"></param>
      <param name="dynamicvol" control="0"></param>
    </list>
  </cmd>
</rx>

Do other AVR models return this information in the node text instead?

JPHutchins commented 3 years ago

@MoshiBin The control attribute is different. It is the receiver's way of specifying whether or not the parameter is settable or not. It may show control="0" and nothing in the tag text because the receiver is off or because the current source is not compatible with Audyssey settings.

Good catch on the int() error!

We would like to utilize the control attribute in the future (like only showing parameters in a HA UI if they are available) but it's a ways off.

JPHutchins commented 3 years ago

@MoshiBin I see your SSDPy project, very cool! I am working on a PR over at https://github.com/flyte/upnpclient/pull/33 and I'd love to know what you think about their UPnP implementation. I have decided to use their upnpclient for a UPnP AV Receiver project but I had to put in the PR when I realized they had no asyncio compatability. Denon's UPnP is a little underwhelming but it does send out notifications of volume changes - so we can see a Home Assistant slider move as I turn the volume knob lol!

MoshiBin commented 3 years ago

Thank you!

Their UPnP implementation looks very comprehensive. Though I'm not sure if they'll accept your PR since it will break py2.7 compatibility.

SSDPy was born from a need to use the IETF flavor of SSDP rather than UPnP (specific use case was discovering Redfish instances, which specifically uses IETF SSDP). The codebase that interacts with it is unfortunately still py2.7-based, though that might change soon. Due to this, I still can't use asyncio freely. Maybe there's a way to only enable it for py3 installations - but I'm guessing we'll have to hack it a little to only import files with async def if we're running on py3.

ol-iver commented 3 years ago

Fixed with this commit https://github.com/scarface-4711/denonavr/commit/b073203039f1bf2a4b45cd48fd4e9ce298f1f34d and added to HA in this PR https://github.com/home-assistant/core/pull/44194