Skip to content

[broadlink] Broadlink binding - #14582

Closed
rlarranaga wants to merge 9 commits into
openhab:mainfrom
rlarranaga:broadlink-binding-4.0.x
Closed

[broadlink] Broadlink binding#14582
rlarranaga wants to merge 9 commits into
openhab:mainfrom
rlarranaga:broadlink-binding-4.0.x

Conversation

@rlarranaga

Copy link
Copy Markdown

A binding that can communicate with the Broadlink family of WiFi-addressable devices, currently comprising:

A1 environmental sensor
RMx Remote IR blasters
SPx WiFi-controlled power switches
MPx WiFi-controlled multi-outlet power strips
Development of this binding was begun some years ago by one Cato Sognen, who unfortunately seems to have stopped development after posting about it here:
https://community.openhab.org/t/broadlink-binding-for-rmx-a1-spx-and-mp-any-interest/22768

Jhon Marshall (@themillhousegroup) reverse-engineered the binding's JAR file and gradually built up an understanding of its operation, fixing a few bugs and adding support for more devices along the way, including bringing it up through the 2.x and 3.x upgrade paths.

As I am a user of the binding and see great value in it, I am looking to help integrate into the OpenHAB codebase, and improving it in the future, when time permits.

There has been significant interest and help provided by interested parties, as evidenced by the numerous threads regarding broadlink in the OpenHAB forum! It would be fantastic to get this binding into the official release to reward their patience :-)

Thanks also to @Skinah who provided invaluable guidance in previous PRs for this binding.
closes #322

This PR is a contiuation of the PR opened in #11049

Signed-off-by: Ricardo Larranaga ricardo.larranaga@gmail.com

Ricardo Larranaga added 9 commits March 11, 2023 14:54
Signed-off-by: Ricardo Larrañaga <ricardo.larranaga@gmail.com>
Signed-off-by: Ricardo Larranaga <ricado.larranaga@gmail.com>
Signed-off-by: Ricardo Larrañaga <ricardo.larranaga@gmail.com>
Signed-off-by: Ricardo Larranaga <ricado.larranaga@gmail.com>
Signed-off-by: Ricardo Larrañaga <ricardo.larranaga@gmail.com>
Signed-off-by: Ricardo Larranaga <ricado.larranaga@gmail.com>
Signed-off-by: Ricardo Larrañaga <ricardo.larranaga@gmail.com>
Signed-off-by: Ricardo Larranaga <ricado.larranaga@gmail.com>
Signed-off-by: Ricardo Larrañaga <ricardo.larranaga@gmail.com>
Signed-off-by: Ricardo Larranaga <ricado.larranaga@gmail.com>
Signed-off-by: Ricardo Larrañaga <ricardo.larranaga@gmail.com>
Signed-off-by: Ricardo Larranaga <ricado.larranaga@gmail.com>
…nhab/binding/broadlink/handler/BroadlinkBaseThingHandler.java

Signed-off-by: Ricardo Larrañaga <ricardo.larranaga@gmail.com>
Signed-off-by: Ricardo Larranaga <ricado.larranaga@gmail.com>
Signed-off-by: Ricardo Larrañaga <ricardo.larranaga@gmail.com>
Signed-off-by: Ricardo Larranaga <ricado.larranaga@gmail.com>
Signed-off-by: Ricardo Larrañaga <ricardo.larranaga@gmail.com>
Signed-off-by: Ricardo Larranaga <ricado.larranaga@gmail.com>
@rlarranaga
rlarranaga requested a review from a team as a code owner March 11, 2023 19:11
@jlaur jlaur added the new binding If someone has started to work on a binding. For a new binding PR. label Mar 11, 2023
@wborn wborn changed the title [broadlink] Broadlink binding 4.0.x [broadlink] Broadlink binding Mar 11, 2023
@Skinah

Skinah commented Mar 25, 2023

Copy link
Copy Markdown
Contributor

@GiviMAD Can you comment on this at all since I saw you were looking to rename the broadlinkthermostat binding to just broadlink here:

#13412

Are there weaknesses in the broadlinkthermostat binding that this newer one solves? Would be good to get some feedback from people that have tried both to determine what is the best way forward. I have no opinion on this as I do not have any of the hardware. Would be good for an agreement to be made on which way it is going to head before V4.0 Stable is released so that any breaking changes can be made at the V4 jump.

@GiviMAD

GiviMAD commented Mar 25, 2023

Copy link
Copy Markdown
Member

Hi, amazing work!

For me this one seems a better option that the renaming of the current one, as it based in a java library that is missing compatibility with a lot of devices. https://github.com/mob41/broadlink-java-api/tree/master (last device added was the RM3mini in 2019).

I thought on porting the https://github.com/mjg59/python-broadlink into a binding as seems more updated and support more devices, but never looked at it.

So for me make sense to have these as the official binding, and if the thermostats can be supported in these one, delete the previous one.

@GiviMAD

GiviMAD commented Mar 25, 2023

Copy link
Copy Markdown
Member

I have an rm3 mini and rm4 mini, I can try the binding with those, but I don't have any thermostat.

@rlarranaga

Copy link
Copy Markdown
Author

Hi, amazing work!

For me this one seems a better option that the renaming of the current one, as it based in a java library that is missing compatibility with a lot of devices. https://github.com/mob41/broadlink-java-api/tree/master (last device added was the RM3mini in 2019).

I thought on porting the https://github.com/mjg59/python-broadlink into a binding as seems more updated and support more devices, but never looked at it.

So for me make sense to have these as the official binding, and if the thermostats can be supported in these one, delete the previous one.

Hi, @Skinah i would like to chip in here, as I have played with both libraries.

The main driver for me to submit this PR to OpepnHAB is that the current library does not appear to work with Broadlink RM 4 Series. When I looked into trying to submit a PR to support them, i realized the protocol is actually handled by the broadlink-java-api library, which has not been maintained for a couple of years. So including support for new devices would have to be done by either getting the new devices supported in the library, or forking the library, adopting it and including support.

Since i only have RM mini 3 and RM mini 4 devices, i cannot really do that. The binding i am proposing already has the code included, and making any suggested changes from last PR and recompiling for OH 4 Seemed like path with less workload.

I really like the current binding, specially the way it manages remote code learning, and i hope to adopt some of the ideas on that in the future for the binding i am proposing.

@GiviMAD , If i can give my 2 cents, i would keep both libraries for the medium term, as they both have their own strengths.

One thing we could do is got to one of the numerous broadlink tickets in the community, and ask for someone with the thermostats to test them?

let me know if you have any questions!

@GiviMAD

GiviMAD commented Mar 25, 2023

Copy link
Copy Markdown
Member

I really like the current binding, specially the way it manages remote code learning, and i hope to adopt some of the ideas on that in the future for the binding i am proposing.

@GiviMAD , If i can give my 2 cents, i would keep both libraries for the medium term, as they both have their own strengths.

Out of curiosity, could you elaborate that part? What are the strengths you see in the previous one?

@GiviMAD

GiviMAD commented Mar 25, 2023

Copy link
Copy Markdown
Member

@rlarranaga maybe the BroadlinkMappingService could make use of org.openhab.core.storage.StorageService to handle the persistence? Just a suggestion, I haven't looked at it enough to know if there is a reason to prefer the current implementation.

@rlarranaga

Copy link
Copy Markdown
Author

Hey @GiviMAD
Regarding your firs question:

Out of curiosity, could you elaborate that part? What are the strengths you see in the previous one?

Both bindings are capable of learning codes through a channel in the thing. The current binding is also capable to automatically save that channel to a file, and at that point, you handle everything from OpenHAB interface. In the PR i am Submitting, the learned remotes get output in the logs, and you need to add them manually to the IR Code file. This is explained in the README.md file.

@rlarranaga maybe the BroadlinkMappingService could make use of org.openhab.core.storage.StorageService to handle the persistence? Just a suggestion, I haven't looked at it enough to know if there is a reason to prefer the current implementation.

It is an idea worth exploring, and it is tied with your first question. I would have to look into StorageService more deeply.
I do like to have the codes available in a raw manipulable way. If you are dealing with IR codes a lot, it is handy.

@GiviMAD

GiviMAD commented Mar 25, 2023

Copy link
Copy Markdown
Member

Both bindings are capable of learning codes through a channel in the thing. The current binding is also capable to automatically save that channel to a file, and at that point, you handle everything from OpenHAB interface. In the PR i am Submitting, the learned remotes get output in the logs, and you need to add them manually to the IR Code file. This is explained in the README.md file.

I did that part, I can give you assistance if you need any, or even made a PR to your repo if you prefer that, but in that case I will be the one needing some assistance.

It is an idea worth exploring, and it is tied with your first question. I would have to look into StorageService more deeply. I do like to have the codes available in a raw manipulable way. If you are dealing with IR codes a lot, it is handy.

The instances of the StorageService is are persisted into json files in the "jsondb" folder in the userdata folder and have a backup functionality, which is cool, but if you think the users will want to edit the IR codes by hand usually maybe not the best solution as they will need to turn down their openhab server, and use the correct format, so maybe not the most confortable solution for this case.

@rlarranaga

rlarranaga commented Mar 25, 2023 via email

Copy link
Copy Markdown
Author

@GiviMAD

GiviMAD commented Mar 25, 2023

Copy link
Copy Markdown
Member

The main difference between the two services is that broadlinkthermostat
saves codes for all remotes on a directory, each code on one file (you know
this, you wrote it 😄)

Yes, I probably would have used the StorareService, but I didn't know about it existence back them, that is why I wanted to bring it to the conversation, in case it was the same case.

The simplest solution moving forward might be to:
1- Allow only one file for storing codes for all things
2- replace the mapper service with the storageservice.
3- Add the "save ir code" channel to the remote thing type.
Do you thing this is a good way to go, or do you think we should keep de
functionality of allowing different remote code files per thing?

For me it sounds like a great idea.

I think that having a single shared storage for the ir values will make it more easy to use in some situations. And the "save ir" channel sound like a better idea than editing the storage manually, I didn't think about that.

@rlarranaga

rlarranaga commented Mar 28, 2023 via email

Copy link
Copy Markdown
Author

@GiviMAD

GiviMAD commented Mar 28, 2023

Copy link
Copy Markdown
Member

If you push more commits to the branch in the PR, it will be updated as well, no more actions needed from your side.

I think is a good idea to include it in this PR to avoid future breaking changes as you said.

I can test the binding with an Rm3 device when the change is done.

@rlarranaga

rlarranaga commented Mar 28, 2023 via email

Copy link
Copy Markdown
Author

@dilyanpalauzov

Copy link
Copy Markdown
Contributor

Will the Broadcom HTS2 sensor (temperature and humidity measurement via normal USB transit cable) be supported?

@rlarranaga

rlarranaga commented Apr 10, 2023 via email

Copy link
Copy Markdown
Author

<parent>
<groupId>org.openhab.addons.bundles</groupId>
<artifactId>org.openhab.addons.reactor.bundles</artifactId>
<version>4.0.0-SNAPSHOT</version>

@wborn wborn Jul 24, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
<version>4.0.0-SNAPSHOT</version>
<version>4.2.0-SNAPSHOT</version>

@lsiepel

lsiepel commented Oct 30, 2023

Copy link
Copy Markdown
Contributor

@rlarranaga Do you need anything to proceed, besides time ;-)

@openhab-bot

Copy link
Copy Markdown
Collaborator

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

https://community.openhab.org/t/oh-4-0-4-broadlink-binding-rm3-mini-error/152173/5

@lsiepel lsiepel added the awaiting feedback Awaiting feedback from the pull request author label Dec 29, 2023
@lsiepel

lsiepel commented Jan 14, 2024

Copy link
Copy Markdown
Contributor

@lsiepel lsiepel closed this Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting feedback Awaiting feedback from the pull request author new binding If someone has started to work on a binding. For a new binding PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Broadlink RM Pro Binding

8 participants