search

crowbarz / aiopioneer

Pioneer AVR API (asyncio)
Apache License 2.0
8 stars 6 forks source link

Add support for new parameters #4

Closed pantherale0 closed 1 year ago

pantherale0 commented 1 year ago

Opening this PR to add support for new parameters and functions that were not in previous versions.

Majority of video functions, amp functions, all tone functions and listening modes will be added. Below is a check list that I need to work through to get all the responses decoded, then I will work through this list again to implement any functions that currently are not implemented. This is all taken from official Pioneer documentation

I have loads more changes for ha-pioneer_async lined up to include support for all of this.

crowbarz commented 1 year ago

Looking and sounding great. I got the basic functionality (on/off, input switching, volume) that I needed working and unfortunately everything else went on the backlog. The next items on my list was decoding the audio and video parameters to be able to detect stereo vs multichannel audio, and being able to tune the radio frequency - both of which are already on your list :)

pantherale0 commented 1 year ago

Thanks, the biggest one for me was the listening modes, the remote for my AVR doesn't work very well anymore (despite replacing batteries) and the iControl app has completely stopped working on my phone, while I was at it I thought to myself "well, some other features in the app are quite useful" so decided to just go through all the docs and serial commands / responses.

I've just finished audio / video information decoding but I need some help as at the moment it is only able to if we sent ?AST or ?VST, which I need to send every time we receive input changes or power state changes.

Other solution to this (maybe) is that at least on my AVR I get a response starting with AUA / AUB when the source changes but I've found no documentation on what that actually is yet.

crowbarz commented 1 year ago

Sending another command when the parser (run from the connection listener task) detects a state change is a little tricky - you need to schedule a separate task to call send_command() to avoid blocking the connection listener task. This is currently done in _connection_listener() to schedule bounce_volume() when AVR power-on is detected if PARAM_POWER_ON_VOLUME_BOUNCE is enabled, you should be able to do something similar when an input or power state change is detected.

pantherale0 commented 1 year ago

Using inspiration from the bounce_volume() functions and how that's scheduled in, I've added a command queue, which can be used to schedule in commands to run after processing the response by adding commands_to_queue.add("COMMAND HERE") in the _parse_response() function.

I don't think AST / VST will be sent in the serial session if that state changes on the AVR itself (IE, HDMI input goes from stereo to multi channel etc.)

Tuner functions complete, a few actions I haven't added as other functions take care of what they would do, list below:

No specific iPod functionality added, apart from just mapping the commands and being able to send those commands, might be useful in HA where we can have play/pause buttons etc. I'm not able to test this though as I don't own an iPod.

pantherale0 commented 1 year ago

I think this is ready now, there a probably some places that could be improved on, happy to change where needed.

These two commands have been added now:

TUNER PRESET INCREMENT TUNER PRESET DECREMENT

They are used as previous and next commands for the media controls...

I need a place to document the media controls though, would you be happy to enable to wiki feature in this repo and that could then be used to store the Python API docs perhaps? Currently it doesn't return playback information, however I do have the docs for that but I'm not sure if the AVRs will automatically send playback info when that state changes or if I'll have to schedule more frequent updates for those.

crowbarz commented 1 year ago

I enabled the wiki, but I think you need to first have submitted to this repo in order for you to update the wiki.

I started reviewing the PR, it works with my VSX-930 so that's a great start :) I have a few initial general comments and will add additional comments to the code as I go (may take a while, there's a lot to digest):

I've also updated cli.py to add a few commands that dump the various new attributes, and will submit that to your fork shortly.

pantherale0 commented 1 year ago

There's an unfortunate clash of terminology here as I've used "parameter" to refer to operational attributes that could differ between different AVR models, such as maximum possible volume or a flag for a feature that exists only for some AVR models. But I note that Pioneer also uses "parameter" to refer to command and response arguments (as well as audio/video parameter!). I guess this might be the rationale for the naming of PARAM_MEDIA_CONTROL_SOURCES, PARAM_MEDIA_CONTROLCOMMANDS, etc? To avoid conflation between these two uses, I'd suggest just dropping the PARAM prefix for these, they seem to me to just be dicts used to translate response values to something more meaningful. It would be good for those dicts to be moved to a separate file, eg. const.py. This could be a good future home for PIONEER_COMMANDS to reduce pioneer_avr.py by ~700 lines, down from 3200+ lines (a bit long I think!) though let's leave that change for a separate PR.

Agree with all of this, I've updated now and dropped PARAM, we can move a lot out to a const.py file as suggested later on.

A full update takes about 5 seconds on my AVR as it queries a lot of attributes now. It may be worth gating the updates behind parameters for some high level categories (eg. tone, amp, tuner, channel_level, dsp, video, audio) that can be turned off by default and turned on only for specific models via model specific default parameters or user level parameter overrides. This would minimise backwards compatibility and reduce impact to AVRs that may not have such features or be able to handle all the new queries. These groups would probably also be useful when exposing the attributes to HA, as I'd ideally prefer to avoid having to update the HA integration whenever more attributes are added.

Noted, I've updated now and added the following user configurable parameters:

PARAM_DISABLE_AUTO_QUERY - disables all queries when _update_zone is called. PARAM_ENABLED_FUNCTIONS - provides the ability to only query certain "functions", in future we can use this against some of the set commands, but for now it splits the PIONEER_COMMAND key on an underscore, and checks if the second item in that array is in PARAM_ENABLED_FUNCTIONS.

Currently the following is supported for PARAM_ENABLED_FUNCTIONS:

crowbarz commented 1 year ago

I made some changes to the command queue functions to minimise repeated commands being sent to the AVR, and also migrated the volume bouncer to use the command queue instead, can you see if either of these impact your changes.

I haven't tested the new set_* functions yet, they can be tested later. VScode/pylint is also complaining about a few other issues but I think these can also be addressed later on.

The above notwithstanding I think this PR is pretty much ready to merge, after which I can push a beta release to PyPi and test further within my prod environment, and also work on the HA integration to expose the additional AVR info and functions to HA. Thanks again @11harveyj for your efforts

pantherale0 commented 1 year ago

Ok, great. I'll test these changes later on and feedback :)

pantherale0 commented 1 year ago

Everything is still working ok for me, my HA instance is pending an update so when I update that I'll test those changes in HA too (changed the manifest.json to point to my fork).