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

[sonos] Support for more audio streams through the HTTP audio servlet #15116

Merged
merged 2 commits into from
Jul 2, 2023
Merged

[sonos] Support for more audio streams through the HTTP audio servlet #15116

merged 2 commits into from
Jul 2, 2023

Conversation

lolodomo
Copy link
Contributor

Related to #15113

Signed-off-by: Laurent Garnier lg.hc@free.fr

@lolodomo lolodomo added the enhancement An enhancement or new feature for an existing add-on label Jun 18, 2023
@lolodomo lolodomo requested a review from kgoderis as a code owner June 18, 2023 21:22
@lolodomo
Copy link
Contributor Author

@dalgwen : it is a first audio sink update to use AudioSinkSync. I kept an asynchronous handling for URLAudioStream. Any audio stream except URLAudioStream is now handled using the new method audioHTTPServer.serve.

@lolodomo lolodomo mentioned this pull request Jun 18, 2023
Closed
13 tasks
@lolodomo lolodomo changed the title [sonos] Audio sink supporting more audio streams [sonos] Support for more audio streams through the HTTP audio servlet Jun 18, 2023
@lolodomo
[sonos] Audio sink supporting more audio streams
b977156 
Related to openhab#15113

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@dalgwen
Copy link
Contributor

dalgwen commented Jul 1, 2023

Hello,
I can't test and maybe I miss something, but I don't understand why it extends AudioSinkSync ?
In the processSynchronously() you use the httpAudioServlet, which is inherently async.
You should use the AudioSinkSync class and its method only if processSynchronously() returns when the sound is fully played.

@lolodomo
Copy link
Contributor Author

lolodomo commented Jul 1, 2023

This one was of course fully tested by me.

I don't understand why it extends AudioSinkSync ?
In the processSynchronously() you use the httpAudioServlet, which is inherently async.
You should use the AudioSinkSync class and its method only if processSynchronously() returns when the sound is fully played.

What is synchronous is the handling by handler.playNotificationSoundURI.

@lolodomo
Close audio stream + exception message
64cf967 
Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@dalgwen
Copy link
Contributor

dalgwen commented Jul 1, 2023

What is synchronous is the handling by handler.playNotificationSoundURI.

OK, I get it now, thanks. LGTM

@lolodomo
Copy link
Contributor Author

lolodomo commented Jul 1, 2023

@openhab/add-ons-maintainers : I believe this PR is ready for a merge.

@kaikreuzer kaikreuzer merged commit 070de81 into openhab:main Jul 2, 2023
@kaikreuzer kaikreuzer added this to the 4.0 milestone Jul 2, 2023
@lolodomo lolodomo deleted the sonos_sink branch July 2, 2023 09:40
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Jul 8, 2023
@lolodomo @markus7017
[sonos] Support for more audio streams through the HTTP audio servlet (…
fd6b736 
…openhab#15116)

* [sonos] Audio sink supporting more audio streams

Related to openhab#15113

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Aug 9, 2023
@lolodomo @matchews
[sonos] Support for more audio streams through the HTTP audio servlet (…
e608405 
…openhab#15116)

* [sonos] Audio sink supporting more audio streams

Related to openhab#15113

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
Signed-off-by: Matt Myers <mmyers75@icloud.com>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@lolodomo @austvik
[sonos] Support for more audio streams through the HTTP audio servlet (…
160eb07 
…openhab#15116)

* [sonos] Audio sink supporting more audio streams

Related to openhab#15113

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaikreuzer kaikreuzer kaikreuzer approved these changes

@kgoderis kgoderis Awaiting requested review from kgoderis kgoderis 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.0
Development

Successfully merging this pull request may close these issues.
3 participants
@lolodomo @dalgwen @kaikreuzer