[snapcast] Snapcast Binding initial contribution #4189

StbX · 2018-11-02T15:52:48Z

This is a new binding to integrate snapcast.

Snapcast is a multi-room client-server audio player, where all clients are time synchronized with the server to play synced audio.
This binding allows to control the snapcast clients (change volume, mute, select stream).

An compiled JAR is available at: https://github.com/StbX/openhab2-addons/releases

kaikreuzer · 2018-11-03T22:04:47Z

Snapcast looks like a very cool project for DIY multi-room audio 👍 .

FTR: Every change to this PR will automatically also be deployed to https://openhab.jfrog.io/openhab/libs-pullrequest-local/org/openhab/binding/org.openhab.binding.snapcast/2.4.0-SNAPSHOT/, so there's no need for you to provide a manually build jar anywhere else!

cguedel · 2018-11-28T20:31:42Z

This looks very cool. However, I was not able to control volume or play/pause a client using the configuration example you have provided. It seems like the system:volume channel is not found and there is no player channel at all?

StbX · 2018-11-29T06:54:00Z

@cguedel are you already using the unreleased openHAB 2.4?
I’m using for the description of the volume and mute channels global system-types. As far da I know, these system-types are new in the version 2.4.

Signed-off-by: Steffen Brandemann <steffen+openhab@dakloetz.de>

Signed-off-by: Steffen Brandemann <steffen+openhab@dakloetz.de> (github: StbX)

openhab-bot · 2019-02-20T06:55:15Z

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

https://community.openhab.org/t/snapcast-binding/25551/20

J-N-K · 2019-09-22T16:08:57Z

@StbX, do you intend to continue this work?

StbX · 2019-09-30T18:19:58Z

@J-N-K Yes, I would like to continue this work.
I'm using this addon in my (currently outdated) openHAB.

martinvw

@StbX thanks for your continued work.

I added some comments, please let us know when you need any help.

martinvw · 2020-01-13T10:03:38Z

addons/binding/org.openhab.binding.snapcast/.project

@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8"?>


One of the major steps you have to do now is the BND migration more info:

https://community.openhab.org/t/tutorial-migrate-your-binding-to-the-maven-bnd-based-build-system/81389

martinvw · 2020-01-13T10:10:37Z

addons/binding/org.openhab.binding.snapcast/ESH-INF/thing/channels.xml

+	xmlns:thing="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0"
+	xsi:schemaLocation="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0 http://eclipse.org/smarthome/schemas/thing-description-1.0.0.xsd">
+
+	<channel-type id="serverStreams">


I think that these channels would make more sense if they are provided as a selectable list of Options, see for example the Spotify binding:

https://github.com/openhab/openhab-addons/blob/2.5.x/bundles/org.openhab.binding.spotify/src/main/java/org/openhab/binding/spotify/internal/handler/SpotifyDynamicStateDescriptionProvider.java

Can you take a look at that? If that does not match what you need please explain your usecase.

martinvw · 2020-01-13T10:12:33Z

addons/binding/org.openhab.binding.snapcast/ESH-INF/thing/channels.xml

+		<state readOnly="true" />
+	</channel-type>
+
+	<channel-type id="clientName">


What is the use case for this client-name? Does it make sense that it is read/write.

martinvw · 2020-01-13T10:12:58Z

addons/binding/org.openhab.binding.snapcast/ESH-INF/thing/channels.xml

+		<state readOnly="false" />
+	</channel-type>
+
+	<channel-type id="clientLatency">


Some of these channels might make sense as properties wdyt?

martinvw · 2020-01-13T10:13:46Z

addons/binding/org.openhab.binding.snapcast/ESH-INF/thing/client.xml

+			<channel id="volume" typeId="system.volume" />
+			<channel id="mute" typeId="system.mute" />
+			<channel id="latency" typeId="clientLatency" />
+			<channel id="stream" typeId="streamId" />


To what does this stream refer? Is there also something like a stream name?

martinvw · 2020-01-13T10:32:07Z

...pcast/src/main/java/org/openhab/binding/snapcast/internal/handler/SnapcastClientHandler.java

+                break;
+            case CHANNEL_CLIENT_VOLUME:
+                if (command instanceof RefreshType) {
+                    clientProtocolHandler.updateVolumn(clientId);


Typo volumn -> volume

martinvw · 2020-01-13T10:40:38Z

...napcast/src/main/java/org/openhab/binding/snapcast/internal/protocol/AbstractController.java

+     * @param origin the origin state
+     * @param update the new state
+     */
+    protected <O extends Identifiable> void mergeThingState(O origin, O update) {


Is this really needed? What is the exact usecase for this?

martinvw · 2020-01-13T10:44:15Z

...napcast/src/main/java/org/openhab/binding/snapcast/internal/protocol/SnapcastController.java

+                try {
+                    sh.openSocket();
+                } catch (IOException e) {
+                    serverController.disconnected();


Please make sure to update the status of thing Things

martinvw · 2020-01-13T10:47:33Z

...napcast/src/main/java/org/openhab/binding/snapcast/internal/protocol/SnapcastController.java

+                        try {
+                            openSocket();
+                        } catch (IOException e) {
+                            Thread.sleep(5000);


Is this Thread.sleep really needed, can you maybe just reschedule a thread?

martinvw · 2020-01-13T10:48:16Z

...napcast/src/main/java/org/openhab/binding/snapcast/internal/protocol/SnapcastController.java

+                } catch (InterruptedException e) {
+                    running = false;
+                } catch (IOException e) {
+                    logger.error("snapcast connection error", e);


In E.4 of the coding guidelines we listed what kind of logging level should be used when. Error should only be used:

error logging should only be used to inform the user that something is tremendously wrong in his setup, the system cannot function normally anymore, and there is a need for immediate action.

Please change this to use the correct logging levels and if possible change the ONLINE/OFFLINE status of thing / bridge.

Hilbrand · 2020-08-27T12:13:29Z

Closing due to long time no activity. If you still want to have this merged feel free to reopen or create a new pr.

wborn added the new binding If someone has started to work on a binding. For a new binding PR. label Nov 2, 2018

StbX force-pushed the snapcast branch 2 times, most recently from 3886c51 to 29343e6 Compare November 27, 2018 23:58

wborn changed the title ~~[snapcast] Initial contribution~~ [snapcast] Snapcast Binding initial contribution Dec 18, 2018

snapcast binding: initial contribution

27de5d0

Signed-off-by: Steffen Brandemann <steffen+openhab@dakloetz.de>

StbX force-pushed the snapcast branch from 29343e6 to 27de5d0 Compare February 19, 2019 20:29

StbX requested a review from a team as a code owner February 19, 2019 20:29

Steffen Brandemann added 2 commits February 19, 2019 22:53

[Snapcast] add binding to feature.xml

c4f7133

Signed-off-by: Steffen Brandemann <steffen+openhab@dakloetz.de> (github: StbX)

[Snapcast] updated for version 2.5.0

02607df

Signed-off-by: Steffen Brandemann <steffen+openhab@dakloetz.de> (github: StbX)

J-N-K added the awaiting feedback label Sep 22, 2019

martinvw changed the base branch from master to 2.5.x January 13, 2020 09:56

martinvw reviewed Jan 13, 2020

View reviewed changes

martinvw removed the awaiting feedback label Jan 13, 2020

cpmeister added the awaiting feedback label Mar 17, 2020

J-N-K added stale As soon as a PR is marked stale, it can be removed 6 months later. and removed awaiting feedback labels May 9, 2020

kaikreuzer force-pushed the 2.5.x branch from 536fc42 to 3ab1af1 Compare June 21, 2020 16:12

Hilbrand closed this Aug 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[snapcast] Snapcast Binding initial contribution #4189

[snapcast] Snapcast Binding initial contribution #4189

StbX commented Nov 2, 2018

kaikreuzer commented Nov 3, 2018

cguedel commented Nov 28, 2018

StbX commented Nov 29, 2018 •

edited

openhab-bot commented Feb 20, 2019

J-N-K commented Sep 22, 2019

StbX commented Sep 30, 2019 •

edited

martinvw left a comment

martinvw Jan 13, 2020

martinvw Jan 13, 2020

martinvw Jan 13, 2020

martinvw Jan 13, 2020

martinvw Jan 13, 2020

martinvw Jan 13, 2020

martinvw Jan 13, 2020

martinvw Jan 13, 2020

martinvw Jan 13, 2020

martinvw Jan 13, 2020

Hilbrand commented Aug 27, 2020

[snapcast] Snapcast Binding initial contribution #4189

[snapcast] Snapcast Binding initial contribution #4189

Conversation

StbX commented Nov 2, 2018

kaikreuzer commented Nov 3, 2018

cguedel commented Nov 28, 2018

StbX commented Nov 29, 2018 • edited

openhab-bot commented Feb 20, 2019

J-N-K commented Sep 22, 2019

StbX commented Sep 30, 2019 • edited

martinvw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hilbrand commented Aug 27, 2020

StbX commented Nov 29, 2018 •

edited

StbX commented Sep 30, 2019 •

edited