search

miracle2k / onkyo-eiscp

Control Onkyo A/V receivers over the network; usuable as a script, or as a Python library.
MIT License
466 stars 110 forks source link

command_to_iscp cannot process integers and incorrectly splits strings #127

Closed davidspek closed 3 years ago

davidspek commented 3 years ago

While trying to fix the handling of subwoofer-temporary-level I found a rather peculiar issue regarding command_to_iscp. While receiver.command('master-volume=160'), receiver.command('master-volume 160'), receiver.command('volume=160') and receiver.command('volume 160') all work correctly. Executing the command according to the documentation using receiver.command('master-volume', 110, 'main') will result in the following error:

/opt/conda/lib/python3.8/site-packages/eiscp/core.py in command_to_iscp(command, arguments, zone)
    206     # providing the user an API with multiple arguments (TODO: not
    207     # currently supported).
--> 208     argument = arguments[0]
    209 
    210     # 1. Consider if there is a alias, e.g. level-up for UP.

TypeError: 'int' object is not subscriptable

Alternatively, when using passing the volume level as a string the conversion to hexadecimal fails for double digit numbers. receiver.command('master-volume', '9', 'main') results in response ('master-volume', 9) receiver.command('master-volume', '10', 'main') results in response ('master-volume', 1)

I had already noticed this while trying to debug what is causing subwoofer-temporary-level to return N/A. command_to_iscp('subwoofer-temporary-level', '9', zone='main') returns 'SWL09' command_to_iscp('subwoofer-temporary-level', '10', zone='main') returns 'SWL01' command_to_iscp('subwoofer-temporary-level', '15', zone='main') returns 'SWL01' command_to_iscp('subwoofer-temporary-level', '20', zone='main') returns 'SWL02'

As you can see, the command_to_iscp funciton is incapable of handling integers, and for strings it is incorrectly splitting them resulting in an incorrect hexadecimal value being created.

davidspek commented 3 years ago

I have gotten further with fixing this issue by properly handling integers. Passing command_to_iscp('subwoofer-temporary-level', '20', zone='main') now returns 'SWL14' as expect. There was another error in ValueRange where the highest value was being removed, thus resulting in an error when command_to_iscp('subwoofer-temporary-level', '24', zone='main') or command_to_iscp('master-volume', '200', zone='main') was called. This has now been fixed so the actual maximum values are present in ValueRange. Once I fix the handling of negative integers and have subwoofer-temporary-level working I will create a pull request.