-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[linkplay] Initial Contribution #19554
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Dan Cunningham <[email protected]>
|
Yes, there is another Linkplay binding on the marketplace...... And no, i am not trying to make a habit of creating competing bindings out there ...... I promise 🤞 In this case i did try reaching out to Michael, but did not get a response. The existing binding has been untouched for a long time, and the author's github accounts looks like they have moved to another home automation system, so i don't think its a problem. This version is fundamentally different in that is was built around UPnP for realtime updates, then i layered on the HTTP api when needed, so there's no polling and and it behaves very similar to the Linkplay/WiiM apps themselves. Also it has strong group support tailored for easy use in our UIs. I have 4 WiiM streamers that have replaced my 4 aging Sonos Connect boxes i am using this with. |
Signed-off-by: Dan Cunningham <[email protected]>
There was a problem hiding this 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 introduces a new binding for LinkPlay-based Wi-Fi audio devices. LinkPlay is a popular audio module platform used by various manufacturers. The binding enables control of playback, volume, input sources, equalizer settings, and multi-room audio grouping through a combination of UPnP and HTTP API communications.
Key changes:
- Adds complete LinkPlay binding with UPnP discovery support
- Implements multi-room audio group management functionality
- Provides AudioSink integration for audio streaming
Reviewed Changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| bundles/pom.xml | Adds linkplay module to build |
| bundles/org.openhab.binding.linkplay/pom.xml | Maven configuration for new binding |
| bundles/org.openhab.binding.linkplay/src/main/resources/OH-INF/thing/thing-types.xml | Channel and thing type definitions |
| bundles/org.openhab.binding.linkplay/src/main/resources/OH-INF/i18n/linkplay.properties | Internationalization strings |
| bundles/org.openhab.binding.linkplay/src/main/java/.../LinkPlayHandlerFactory.java | Factory for creating handlers and managing UPnP device registry |
| bundles/org.openhab.binding.linkplay/src/main/java/.../group/*.java | Multi-room group management service and interfaces |
| bundles/org.openhab.binding.linkplay/src/main/java/.../client/http/*.java | HTTP API client and DTOs for LinkPlay devices |
| bundles/org.openhab.binding.linkplay/src/main/java/.../client/upnp/*.java | UPnP communication, XML parsing, and command execution |
| bundles/org.openhab.binding.linkplay/src/main/java/.../discovery/*.java | UPnP discovery participant implementation |
| bundles/org.openhab.binding.linkplay/README.md | Complete binding documentation with examples |
| bom/openhab-addons/pom.xml | Adds binding to BOM dependencies |
| CODEOWNERS | Registers code owner for the binding |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...linkplay/src/main/java/org/openhab/binding/linkplay/internal/group/LinkPlayGroupService.java
Outdated
Show resolved
Hide resolved
...ding.linkplay/src/main/java/org/openhab/binding/linkplay/internal/client/upnp/UpnpEntry.java
Outdated
Show resolved
Hide resolved
...play/src/main/java/org/openhab/binding/linkplay/internal/client/http/LinkPlayHTTPClient.java
Show resolved
Hide resolved
....binding.linkplay/src/main/java/org/openhab/binding/linkplay/internal/LinkPlayAudioSink.java
Outdated
Show resolved
Hide resolved
....binding.linkplay/src/main/java/org/openhab/binding/linkplay/internal/LinkPlayAudioSink.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Dan Cunningham <[email protected]>
There was a problem hiding this 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 52 out of 53 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bundles/org.openhab.binding.linkplay/src/main/resources/OH-INF/thing/thing-types.xml
Outdated
Show resolved
Hide resolved
bundles/org.openhab.binding.linkplay/src/main/resources/OH-INF/i18n/linkplay.properties
Outdated
Show resolved
Hide resolved
...linkplay/src/main/java/org/openhab/binding/linkplay/internal/group/LinkPlayGroupService.java
Outdated
Show resolved
Hide resolved
...linkplay/src/main/java/org/openhab/binding/linkplay/internal/group/LinkPlayGroupService.java
Outdated
Show resolved
Hide resolved
...play/src/main/java/org/openhab/binding/linkplay/internal/client/http/LinkPlayHTTPClient.java
Show resolved
Hide resolved
Signed-off-by: Dan Cunningham <[email protected]>
Signed-off-by: Dan Cunningham <[email protected]>
Signed-off-by: Dan Cunningham <[email protected]>
Signed-off-by: Dan Cunningham <[email protected]>
Signed-off-by: Dan Cunningham <[email protected]>
There was a problem hiding this 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 54 out of 55 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <channel-type id="outputHardwareMode"> | ||
| <item-type>Number</item-type> | ||
| <label>Output Hardware Mode</label> | ||
| <description>Active audio output (SPDIF, AUX, COAX)</description> | ||
| <state> | ||
| <options> | ||
| <option value="1">SPDIF</option> | ||
| <option value="2">AUX</option> | ||
| <option value="3">COAX</option> | ||
| </options> | ||
| </state> | ||
| </channel-type> |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The outputHardwareMode channel type in thing-types.xml uses Number type with numeric options (1, 2, 3), but the i18n file defines state options with string keys (SPDIF, AUX, COAX). This inconsistency will prevent the i18n labels from being applied. Either change the XML options to use string values matching the i18n keys, or update the i18n keys to match the numeric values.
| channel-type.linkplay.trackPosition.label = Track Position | ||
| channel-type.linkplay.trackPosition.description = Current track position | ||
| channel-type.linkplay.trackSource.label = Track Source | ||
| channel-type.linkplay.trackSource.description = Current track source (Pandora, Spotify, etc..) |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected 'etc..' to 'etc.'.
| public static LoopMode fromCode(int code) { | ||
| for (LoopMode m : values()) { | ||
| if (m.code == code) { | ||
| return m; | ||
| } | ||
| } | ||
| return null; | ||
| } |
Copilot
AI
Nov 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Dan Cunningham <[email protected]>
lsiepel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this binding. Looked at 42/55 files, mainly dto's and metadata files. Handlers will be next.
| @NonNullByDefault | ||
| public class LinkPlayConnectionUtils { | ||
|
|
||
| public static @Nullable Integer testConnection(HttpClient httpClient, String host) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add javadoc to public utility methods
| } catch (Exception ignored) { | ||
| // Continue to next port |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about interruptexception?
| + Character.digit(hex.charAt(i + 1), 16)); | ||
| } | ||
| return new String(data, StandardCharsets.UTF_8); | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this and other files a see the generic Exception is caught, it would be prefered to catch the specific exception at all of those places.
| */ | ||
| @NonNullByDefault | ||
| public interface LinkPlayUpnpDeviceListener { | ||
| void updateDeviceConfig(RemoteDevice device); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add javadoc to all interface methods
| void addDeviceListener(String udn, LinkPlayUpnpDeviceListener listener); | ||
|
|
||
| void removeDeviceListener(String udn); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add javadoc to all interface methods.
| <channel-type id="multiroomActive"> | ||
| <item-type>Switch</item-type> | ||
| <label>Multi-room Active</label> | ||
| <description>Indicates if the device is part of a group</description> | ||
| <state readOnly="true"/> | ||
| </channel-type> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applies to all channel types, please provide semantic tags. (Point+Property)
| <thing-type id="player"> | ||
| <label>LinkPlay Player</label> | ||
| <description>Represents a single LinkPlay-based audio player</description> | ||
| <channel-groups> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a semenatic equipment tag
| <channel id="source" typeId="presetSource"/> | ||
| <channel id="pic-url" typeId="presetPicUrl"/> | ||
| <channel id="pic" typeId="presetPic"/> | ||
| <channel id="play" typeId="presetPlay"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applies to all channel id's ( i think they are good) and channel type id's. Please stick to the naming co9nvention lower-case-hyphen
This integrates audio devices based on the LinkPlay platform. LinkPlay is a popular Wi-Fi audio module used by many manufacturers. This binding uses both UPnP communication for realtime events, meta data, etc.. and the LinkPlay HTTP API for control, group management, etc...
This is dependent on openhab/openhab-core#5095 which was just merged.