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.

Sign up for GitHub

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

Jump to bottom

[denonmarantz] Add HTTP protocol support for newer receivers #16748

Merged
merged 2 commits into from
May 25, 2024
Merged

[denonmarantz] Add HTTP protocol support for newer receivers #16748

merged 2 commits into from
May 25, 2024

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented May 11, 2024 •
edited

The API call for fetching main zone is now auto-detected. Newer receivers will return HTTP status 403 for the legacy API calls, in which case an alternative API call is now used.

Notes:

  • Surround mode is different between Telnet and HTTP AppCommand.xml GetSurroundModeStatus - e.g. "DOLBY AUDIO-DSUR" vs. "Dolby Audio - Dolby Surround". I don't know what formMainZone_MainZoneXmlStatus.xml returns for older models.
  • Information equivalent to formMainZone_MainZoneXml.xml is missing. This is used to update channel general#power. It seems the logic (for my receiver) is that if any zones are on, this is on, otherwise off. The same logic could be achived from a Group:Switch:OR(ON,OFF) item, but since this channel is updated directly from the receiver, we should not try to replicate that logic in the binding. In the case of HTTP protocol for newer receivers, this value is simply missing.

Resolves #16747

@jlaur jlaur added enhancement An enhancement or new feature for an existing add-on work in progress A PR that is not yet ready to be merged labels May 11, 2024
@jlaur jlaur force-pushed the 16747-denonmarantz-http-protocol branch from 78661b1 to e7706c0 Compare May 11, 2024 22:17
@jlaur jlaur mentioned this pull request May 13, 2024
Merged
@jlaur jlaur force-pushed the 16747-denonmarantz-http-protocol branch 2 times, most recently from 032e3e2 to b7aea90 Compare May 13, 2024 19:31
@jlaur jlaur changed the title [denonmarantz] Add initial HTTP protocol support for newer receivers [denonmarantz] Add HTTP protocol support for newer receivers May 13, 2024
@jlaur
Copy link
Contributor Author

jlaur commented May 13, 2024

Not strictly necessary as either can go first, but I'd prefer to wait for #16757 (which is ready for review now) and rebase towards that.

@jlaur jlaur force-pushed the 16747-denonmarantz-http-protocol branch 4 times, most recently from 53c6c0f to 53cf4c7 Compare May 15, 2024 20:32
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/anyone-using-the-denon-marantz-binding-willing-to-help-with-a-few-tests/156042/5

lolodomo
lolodomo approved these changes May 19, 2024
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@lolodomo
Copy link
Contributor

@jlaur : is it still work in progress or ready for a merge ?

@jlaur
Copy link
Contributor Author

jlaur commented May 20, 2024

is it still work in progress or ready for a merge ?

Still work in progress, I'm awaiting some test results in the community thread in order to clarifiy if there is this direct relation:

  • Port 80: "Legacy" HTTP protocol.
  • Port 8080: Newer (2016+) HTTP protocol.

If this is confirmed, I think the current implementation is acceptable, i.e. deriving the protocol from the port number, which should be fully backwards compatible. If not, I will need to introduce a new configuration parameter for setting the protocol revision, and the auto-configuration logic will also need some tweaking.

@jlaur jlaur force-pushed the 16747-denonmarantz-http-protocol branch from 53cf4c7 to 2419356 Compare May 23, 2024 20:09
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/anyone-using-the-denon-marantz-binding-willing-to-help-with-a-few-tests/156042/19

@jlaur jlaur force-pushed the 16747-denonmarantz-http-protocol branch 4 times, most recently from 2c02584 to ec5bfed Compare May 24, 2024 19:49
@jlaur
Add HTTP protocol support for newer receivers
546700e 
Resolves openhab#16747

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur force-pushed the 16747-denonmarantz-http-protocol branch from ec5bfed to 546700e Compare May 25, 2024 11:00
@jlaur jlaur marked this pull request as ready for review May 25, 2024 11:03
@jlaur jlaur requested a review from jwveldhuis as a code owner May 25, 2024 11:03
@jlaur jlaur removed the work in progress A PR that is not yet ready to be merged label May 25, 2024
@jlaur
Fix exception message
5d981f1 
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
lolodomo
lolodomo approved these changes May 25, 2024
View reviewed changes
Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@lolodomo lolodomo merged commit f8700a9 into openhab:main May 25, 2024
5 checks passed
@lolodomo lolodomo added this to the 4.2 milestone May 25, 2024
@jlaur jlaur deleted the 16747-denonmarantz-http-protocol branch May 25, 2024 12:19
LeeC77 pushed a commit to LeeC77/openhab-addons that referenced this pull request May 27, 2024
@jlaur @LeeC77
[denonmarantz] Add HTTP protocol support for newer receivers (openhab…
c6cc074 
…#16748)

* Add HTTP protocol support for newer receivers

Resolves openhab#16747

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: LeeC77 <lee.charlton00@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lolodomo lolodomo lolodomo approved these changes

@jwveldhuis jwveldhuis Awaiting requested review from jwveldhuis jwveldhuis is a code owner

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.

[denonmarantz] Add HTTP protocol support for newer receivers
3 participants
@jlaur @openhab-bot @lolodomo