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

Properly handle integers and negative integers #128

Closed davidspek closed 3 years ago

davidspek commented 3 years ago

While trying to set the subwoofer temporary level I noticed a few oddities in the handling of arguments. onkyo --host 192.168.xxx.xxx volume=50 would work onkyo --host 192.168.xxx.xxx subwoofer-temporary-level=query would work onkyo --host 192.168.xxx.xxx subwoofer-temporary-level=UP/DOWN would not work onkyo --host 192.168.xxx.xxx subwoofer-temporary-level=24 would also not work.

Along with that, sending raw commands would also not work, such as: onkyo --host 192.168.xxx.xxx SWL+05

The issue with sending RAW commands was caused by: https://github.com/miracle2k/onkyo-eiscp/blob/fdbad74dc5fb959f9b653e21f45b8489e5bf8140/eiscp/script.py#L125 Which would not pass values that contain a + or - in them. This was fixed by using a regex instead.

Next, sending a command in the format receiver.command('subwoofer-temporary-level', 23, 'main') would not work for both subwoofer-temporary-level and master-volume. This was due to all arguments being handled as lists rather than checking if the argument is a list or an integer.

Another small issue was that ValueRange would not contain the last value in the range, preventing setting the volume to the actual maximum.

The final issue was the formatting of the hexadecimal command, which for subwoofer-temporary-level could be positive and negative. Therefor, the range of values is -1E...000...+18 for -15 dB...0 dB...+12 dB (similar to how TFR handles values). To correct for this, if the command prefix is determined to be SWL, an extra 0 will be added for 00 values. Positive values will be prefixed with +, single digit negative values are prefixed with -0 (-1 to -15) and double digit negative values are prefixed with -.

This pull request solves https://github.com/miracle2k/onkyo-eiscp/issues/127 and https://github.com/miracle2k/onkyo-eiscp/issues/104. It can easily be extended to other commands such as center channel temporary level by adding CTL to the prefix check. I have not done this yet, as I would like to investigate what all the commands are that would need to be handled this way. For example, the temporary channel level (which my receiver does not seem to have) will also need these positive and negative dB values.

miracle2k commented 3 years ago

Fantastic thank you.