crowbarz / aiopioneer

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

Move constants into a more suitable location #5

Closed pantherale0 closed 1 year ago

pantherale0 commented 1 year ago

Tidying up parts of #4

crowbarz commented 1 year ago

Creating a new branch for this and various other pending cleanup efforts, eg. make vscode lint happy(ier)

pantherale0 commented 1 year ago

Nice, good idea. Was wondering about separating the parsing out into another file too, but not sure that would work.

Need to refactor the listening modes though as I've found that at least on my AVR, sometimes a multi-channel input can have a regular listening mode ID....

I've tried to find the docs for that issue relating to the VSX-921 amp but can't seem to find it mentioned in anything pioneer publish.. I've sent them an email in hopes they might be able to provide some docs for older AVRs (manufacturing year <= 2012)

pantherale0 commented 1 year ago

Synced with dev, updated set_video_settings to use an array of args like set_dsp_settings... In the future if any functionality is missing it should just be a matter of defining the command in the new commands.py file with the format set_video_<parameter name>

For example: set_video_white_balance

And then another bool arg could be added to whats already being passed in and it will pickup this new command..

More complex structures like some of the dicts will require a little more code so that it converts the argument to the correct key value first, but creating the command in the commands.py file is still the same

pantherale0 commented 1 year ago

And I think we're ready for review, I don't want to make any more big changes against this PR. But I think we've cleaned up a good amount of stuff.

I've noticed a number of issues with listening modes as time has gone when switching between multi-channel (3+ channels) sources and stereo sources. The latest couple of commits adjusts the storage of the listening modes so that different listening modes can be enabled / disabled. I did think about this being a configurable parameter, but I think its best placed as a const as the IDs/names don't change across different AVR generations.

crowbarz commented 1 year ago

There are a few other formatting changes that black still seems to want to make on save, but I'll do this in 0.2.0-cleanup post merge. Will also see if I can remove some of the pylint ignores as well.