Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support multiple instances of Denon AVR #35

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

albaintor
Copy link
Contributor

@albaintor albaintor commented Mar 16, 2024

Tested successfully on my remote (not through simulator) :

  1. Multiple instances management : based on your work on apple TV integration with an intermediate setup flow to add a new device or remove existing device. However I only have one Denon receiver : I was able to start the process to add a new device, it scanned by environment, found my existing device but refused to add the entity as already existing.
  2. Adds new commands : direction pad, enter, back, menus (setup and option menus mapped respectively to settings+menu and contexte menu), info

The embedded library is not up to date so I added the commands directly from the avr.py file.
This new commands work fine on my setup (Denon AVR 4700)

@klada
Copy link

klada commented Mar 16, 2024

I think you are using HTTP control for these commands, even if Telnet control was selected in the integration. But thanks for looking into this.

Copy link
Contributor

@zehnm zehnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for enhancing the features!
I'll test the new commands with my Denon AVR this weekend.

Please reach out first when doing bigger changes, see CONTRIBUTING.md. Related issues:

This PR needs to be split into multiple pull request: one for additional commands and another for multi-device support.

I highly suggest to create a dedicated branch for each PR in your repo and avoid using the main branch. Rebasing and PR changes become much easier.
Preferrably, a PR should only contain one feature in one commit.

requirements.txt Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
.idea/misc.xml Outdated Show resolved Hide resolved
intg-denonavr/setup_flow.py Outdated Show resolved Hide resolved
@albaintor albaintor changed the title Added support for DPAD, menus and multiple instances of Denon AVR Added support multiple instances of Denon AVR Mar 17, 2024
… fork/PR. Only the multi instance feature in this fork
@albaintor
Copy link
Contributor Author

albaintor commented Mar 17, 2024

Hi,
I have made 2 separate PR : this one for multi-instances (and rollbacked there the other changes on additional commands) on the main branch and another one with only the additional commands on a dedicated branch.
And yes I was not used to create branches for PR

About additional commands : I added the 2 modes (telnet and http), whereas only http commands on the previous commit. Tested OK on my setup. However Telnet is supposed to be faster than http commands but it does not feel that way. Maybe http should be preferred

In the future I will only use branches

@albaintor
Copy link
Contributor Author

Hi,
I have applied modifications : code formatting check, merged updates from the last release. This PR is ready to be reviewed (multiple instances support)

@albaintor albaintor requested a review from zehnm March 20, 2024 10:02
@zehnm
Copy link
Contributor

zehnm commented Mar 26, 2024

Thanks for the updated PR!
I'll review and test it later this week, but it will be a bit tricky with only one network capable receiver.

@albaintor
Copy link
Contributor Author

Thanks for the updated PR! I'll review and test it later this week, but it will be a bit tricky with only one network capable receiver.

Hi,
I have only one receiver too however to test it I did the following : I changed the generated config file by modifying the address and then launched the setup flow again to get a second device & entity.

@Kat-CeDe
Copy link

Kat-CeDe commented Jun 5, 2024

Hi,
if it helps I have 1 Denon and 1 Marantz.

Ralf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants