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 additional commands #290

Merged
merged 11 commits into from
May 4, 2024
Merged

Added additional commands #290

merged 11 commits into from
May 4, 2024

Conversation

albaintor
Copy link
Contributor

Added additional commands for AVR : cursor pad, settings and option menu, info as requested in #289

@klada
Copy link
Contributor

klada commented Mar 16, 2024

I think you are always sending HTTP commands, even if the library is in telnet control mode. Or did I miss something here?

@albaintor
Copy link
Contributor Author

I think you are always sending HTTP commands, even if the library is in telnet control mode. Or did I miss something here?

Indeed I am not aware (yet) on how the library works. I added only HTTP commands. Is it the same command names for telnet ?
Also one of the commands expects a reply, is it possible to get a response for telnet commands ?
I will check after this anyway

@albaintor
Copy link
Contributor Author

Hi again, I added corresponding commands for telnet protocole according to this documentation
https://www.heimkinoraum.de/upload/files/product/IP_Protocol_AVR-Xx100.pdf

@albaintor
Copy link
Contributor Author

H @ol-iver, do you intend to merge the PR soon ?
Thank you

@ol-iver
Copy link
Owner

ol-iver commented Apr 2, 2024

I like the idea, thanks for your PR 🚀
This is a private project, so I don't have always enough time for it.
However, I'll check your PR this weekend 🙂

@ol-iver
Copy link
Owner

ol-iver commented Apr 2, 2024

I just enabled the test. You could already start fixing the tests and the linting issues.
In case you want to run them locally, please check the github workflows how you can run linting and unit tests.

@albaintor
Copy link
Contributor Author

Hi, I have updated formatting

Copy link
Owner

@ol-iver ol-iver left a comment

Choose a reason for hiding this comment

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

Found two things.
The first finding might fix the failing tests.

Comment on lines 411 to 421
command_cusor_up=COMMAND_CURSOR_UP,
command_cusor_down=COMMAND_CURSOR_DOWN,
command_cusor_left=COMMAND_CURSOR_LEFT,
command_cusor_right=COMMAND_CURSOR_RIGHT,
command_cusor_enter=COMMAND_CURSOR_ENTER,
command_back=COMMAND_BACK,
command_info=COMMAND_INFO,
command_options=COMMAND_OPTIONS,
command_setup_open=COMMAND_SETUP_OPEN,
command_setup_close=COMMAND_SETUP_CLOSE,
command_setup_query=COMMAND_SETUP_QUERY
Copy link
Owner

Choose a reason for hiding this comment

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

These lines should be added to ZONE2_URLS and ZONE3_URLS too.
This might be the reason why the tests are failing.

@@ -535,6 +535,87 @@ async def async_power_off(self) -> None:
else:
await self.api.async_get_command(self.urls.command_power_standby)

async def async_cursor_up(self) -> None:
Copy link
Owner

Choose a reason for hiding this comment

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

Please call each of your new functions from DenonAVR class like in this example. Otherwise the methods won't be publicly accessible.

@albaintor
Copy link
Contributor Author

Hi, I have added the missing commands (unit testings passed this time) and added public methods

@ol-iver
Copy link
Owner

ol-iver commented Apr 21, 2024

Thanks 🙂
There is one last thing. You too many methods. Those decorated with @run_async_synchronously are deprecated and I removed them with #291 for all other methods.

@albaintor
Copy link
Contributor Author

Hi @ol-iver, I have applied the merge. It should be okay this time :)

@ol-iver
Copy link
Owner

ol-iver commented May 2, 2024

There are still a couple of linting issues 😅
Unless you contributed for the first time, Github does not start the tests automatically. Thus is would make sense if you run the code-format and the integration tests locally.

@albaintor
Copy link
Contributor Author

albaintor commented May 2, 2024 via email

@ol-iver
Copy link
Owner

ol-iver commented May 4, 2024

There was still an issue. I fixed the last one by myself.
Please run the tests locally the next time 😉

The main advantage of telnet is that the properties of your receiver are updated immediately, even when you are pressing buttons on your remote control. When you use HTTP you have to poll for updates with async_update method.

@ol-iver
Copy link
Owner

ol-iver commented May 4, 2024

Thanks for your work 🚀

@ol-iver ol-iver merged commit 0a6cb3d into ol-iver:main May 4, 2024
7 checks passed
@ol-iver ol-iver changed the title Added additional commands Added additional telnet commands Sep 24, 2024
@ol-iver ol-iver changed the title Added additional telnet commands Added additional commands Sep 24, 2024
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.

3 participants