Skip to content

Conversation

@mshameersm
Copy link
Contributor

@mshameersm mshameersm commented Nov 14, 2025

Added support for getting audio format from webAPI.

  • checking could use Denon library's async_get for this

Copilot AI review requested due to automatic review settings November 14, 2025 09:01
@github-actions
Copy link

github-actions bot commented Nov 14, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for retrieving the current audio format from Denon AVR receivers via their web API. The implementation adds a new get_audio_format() method across the abstraction layers.

Key changes:

  • Added get_audio_format() abstract method to the base AudioAmplifier class
  • Implemented the method in DenonAVRController using direct HTTPS requests to the Denon web API
  • Added test coverage for the new functionality

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
framework/core/audioAmplifier/base.py Added abstract method definition for get_audio_format()
framework/core/audioAmplifier/denon_controller.py Implemented get_audio_format() using web API requests and XML parsing
framework/core/audioAmplifierController.py Added wrapper method for get_audio_format()
tests/audioAmp_test.py Added test case for audio format retrieval
Comments suppressed due to low confidence (1)

tests/audioAmp_test.py:92

  • Except block directly handles BaseException.
        except:

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


def __init__(self, host: str):
self.receiver = DenonAVR(host)
self.url = f"https://{host}:10443/"
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't hardcode the port. People could use some strange setup with port forwarding that would change the port.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have updated to pass as parameter

}

def get_audio_format(self):
response = requests.get(f'{self.url}ajax/general/get_config?type=12', verify=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the only way to get this information? Can't it be done using the denonavr library we're using for everything else in the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried all the available APIs. Could not find the current input format after multiple tries with the denonavr package. It was coming in web browser though.

Copilot AI review requested due to automatic review settings November 18, 2025 11:51
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

framework/core/audioAmplifier/denon_controller.py:84

  • Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.
    def get_audio_format(self):

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mshameersm mshameersm marked this pull request as ready for review November 19, 2025 09:57
Copilot AI review requested due to automatic review settings November 19, 2025 09:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

def get_audio_format(self):
try:
# The 'type=12' query parameter requests the configuration from the Denon AVR.
response = requests.get(f'{self.url}ajax/general/get_config?type=12', verify=False, timeout=5)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we please have a docstring/comment explaining why we can't use the denonavr lib. that's used for everything else in the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added.

def get_audio_format(self):
try:
# The 'type=12' query parameter requests the configuration from the Denon AVR.
response = requests.get(f'{self.url}ajax/general/get_config?type=12', verify=False, timeout=5)
Copy link
Contributor

Choose a reason for hiding this comment

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

is verify=false necessary? if so, then can we please explain why

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added


def get_audio_format(self):
try:
# The 'type=12' query parameter requests the configuration from the Denon AVR.
Copy link
Contributor

Choose a reason for hiding this comment

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

what is type=12 and what does it correspond to in the denon api?

Copy link
Contributor Author

@mshameersm mshameersm Nov 24, 2025

Choose a reason for hiding this comment

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

Could not find the corresponding in Denon lib. type=12 in URL returns an xml of configurations from AVR.

Copilot AI review requested due to automatic review settings November 24, 2025 13:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

zghp
zghp previously approved these changes Nov 25, 2025
@mshameersm
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@zghp zghp merged commit f37a8d6 into develop Nov 27, 2025
7 of 8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants