Broadlink binding [4.1.0;4.3.0)

logo

This binding supports a range of home networking devices made by (and occasionally OEM licensed from) Broadlink.

Beta

Changelog

Version 0.2

  • Improved user communication while learning / modifying IR/RF commands
  • Ensure IR/RF commands are properly shared among devices
  • Change storage backend to openhab storage service
  • Include support for RF commands
  • Include UI driven RF/IR commands learning

Version 0.1

  • initial beta release

Resources

Jar file: org.openhab.binding.broadlink-4.2.0-SNAPSHOT.jar

User documentation & source code on github

Alpha

Changelog

Version 0.6

  • Made binding compatible with openhab 4.3.x
  • Includes all refactoring done for the merge with the mainline (yes, we are getting there)
  • Renamed multiple commands and channels to conform to the OpenHAB naming conventions. Hence, you need to recreate your objects when using this version. We decided it is better to have the pain now then later.

Version 0.5

  • Enabled RF for RM Pro models

Version 0.4

  • Improved user communication while learning / modifying IR/RF commands
  • Ensure IR/RF commands are properly shared among devices

Version 0.3

  • Change storage backend to openhab storage service (Alpha)

Version 0.2

  • Include support for RF commands (Alpha)
  • Include UI driven RF/IR commands learning (Alpha)

Version 0.1

  • initial alpha release

Resources

Jar file: org.openhab.binding.broadlink-4.2.0-SNAPSHOT.jar

User documentation & source code on github

Development plans

  • Currently processing review feedback for inclusion into openhab main line.
6 Likes

The name “Broadlink binding” might conflict with the “Broadlink thermostat binding”:

Hello @AntonJansen ,
I forked your repo and added a branch to it named rm4pros_StorageService

I added my changes from your rm4pros branch. You will basically find the following:
-Properties files have been replaced by OpenHAB StorageService.
-Modified (or created, when necessary) lookup, add, replace and remove functions for IR and RF codes. These functions now use the StorageService too.

Modified the README.md file to include commentary on the new storage, the difference with the old storage, and also python script that will migrate an old format file to the new format.

I have 3 things pending addition to this branch:

-Need to fix some minor compile-time warnings

-I am currently playing with the channels, deciding what is the best way to provide an interface to add, remove and modify codes. The main difficulty here is that OpenHAB does not have (that I now of) a channel that will render into free-form text field in the interface, and thus, it is difficult to input the Label you want to use when you are learning a new code.

-When i integrated the StorageService libraries, the unit tests stopped working, so i temporarily removed. I think this is because now that we are using StorageService, these tests have become integration tests and need to be moved to the itest directory. Haven’t tested this yet though.

I will give you a shout when i update those changes.

Thanks!

3 Likes

Hi @ricardol great to hear about all the changes you are making! With respect to the channels, I have been thinking about it as well. For now I choose the easy option by making it just a property of the thing for the command to learn and then use a set in the UI to select the learned command.

However, a better solution, but (much) more work, is to use a system similar to the generic MQTT binding. It allows for the dynamic creation of channels through the UI. Install the add-on, play around with it a bit and let me know what you think of it.

It would be good to first clean up the code (compile warnings, code style stuff etc.) and fix the unit tests. Before we delve into the complexity of getting the channels to work nicely. Could you do a pull request so I can merge in your changes and contribute to the above?

Hi @Bernd_Ritter what name would you propose? I have been thinking about it for a while and I can’t get a better name, so maybe you have some good suggestions :slight_smile:

In my opinion, the other binding should be renamed to “Broadlink Thermostat Binding” in every place.

On the other hand, this binding could be “Broadlink Remote Binding” or something like that…

I believe during the last PR I opened, there was a discussion about removing broadlinkthermostat altogether from OpenHAB…It relies on EOL and unmaintained libraries.

Hey @AntonJansen ,

I agree with everything you mentioned.

I am currently giving it a try to get the tests working again. I need to admit (i haven’t developed in a long time). I am not sure what I am doing, and i haven’t managed to get it to compile.
I will try to get it working this week, so i can make that pull request as clean as possible. If i cannot do that, I might just to the PR without them, and let you know. The compile warnings, etc are simple enough to clean, so i will have that done for sure.

Thanks!

Hi @ricardol that sounds amazing!

Hey @AntonJansen ,

Just updating the thread for progress. I managed to compile the tests successfully, and clean up the warnings. I still have some work to do removing things that are not used anymore and i am away from my OpenHAB instance so i can’t test. The pull request ill be delayed until next weekend, but almost there!

Cheers

1 Like

Hey @AntonJansen
While i am testing the binding, i also pushed to the previously mentioned branch my lastest (and hopefully, last) changes before this gets PR’d to openHAB.
Currently, i am having a number of warnings that i cannot resolve:

[WARNING] openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/main/java/org/openhab/binding/broadlink/internal/handler/BroadlinkRemoteModel4ProHandler.java:[120,29] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/main/java/org/openhab/binding/broadlink/internal/handler/BroadlinkRemoteModel4ProHandler.java:[198,25] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/main/java/org/openhab/binding/broadlink/internal/handler/BroadlinkRemoteModel4ProHandler.java:[243,25] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/main/java/org/openhab/binding/broadlink/internal/handler/BroadlinkRemoteModel4ProHandler.java:[270,9] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/main/java/org/openhab/binding/broadlink/internal/handler/BroadlinkRemoteHandler.java:[128,17] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/main/java/org/openhab/binding/broadlink/internal/handler/BroadlinkRemoteHandler.java:[151,17] Potential null pointer access: this expression has a '@Nullable' type
[WARNING] openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/main/java/org/openhab/binding/broadlink/internal/handler/BroadlinkRemoteHandler.java:[167,9] Potential null pointer access: this expression has a '@Nullable' type

I think 3 of these where already there in your rm4pros branch. They they can be overriden with:

    @SuppressWarnings("null")

But i have left them there for now, so we can discuss if there if a better way to handle them. It looks like a compiler-check error to me.

The code also generates A BUNCH of these:

...
[INFO] /openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/test/java/org/openhab/binding/broadlink/internal/handler/BroadlinkSocketModel3SHandlerTest.java:[102,9] Unsafe interpretation of method return type as '@NonNull' based on substitution 'T=org.openhab.core.thing.binding.@NonNull ThingHandlerCallback'. Declaring type 'org.mockito.Mockito' doesn't seem to be designed with null type annotations in mind
[INFO] /openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/test/java/org/openhab/binding/broadlink/internal/handler/AbstractBroadlinkThingHandlerTest.java:[51,44] Unsafe interpretation of method return type as '@NonNull' based on substitution 'T=org.openhab.binding.broadlink.internal.socket.@NonNull RetryableSocket'. Declaring type 'org.mockito.Mockito' doesn't seem to be designed with null type annotations in mind
[INFO] /openhab2-addons/openhab-addons/bundles/org.openhab.binding.broadlink/src/test/java/org/openhab/binding/broadlink/internal/handler/AbstractBroadlinkThingHandlerTest.java:[52,56] Unsafe interpretation of method return type as '@NonNull' based on substitution 'T=org.openhab.binding.broadlink.internal.socket.@NonNull NetworkTrafficObserver'. Declaring type 'org.mockito.Mockito' doesn't seem to be designed with null type annotations in mind
...

Those, i have no idea how to deal with. The are just INFO level, but because there are a lot of them, it kind of muddies the compile output.

Let me know what your thoughts are before I send you the PR request.

Thanks!

Hi @ricardol, I think we should try to resolve these issues quickly, so I can merge it with the open PR that I have running with the main branch of openhab. This way we will have very little legacy in the field with the old mapping files and have to deal with it’s migration.

As you can tell, the code in the rm4pros branch was very rough and not cleaned up yet. I hold back on it while you were working on it, as to reduce potential merge conflicts.

I checked out your code and indeed some supressWarnings does the trick for the compiler warnings. There are also some style warnings we need to take care off. For the Mockito test INFO warnings, I have no idea :frowning: It seems reading the documentation that Mockito does not use null values, but rather has default return types (e.g. empty list, false, etc.).

Moving forward, I suggest the following approach. You create a pull request to me and I merge it with my rm4pro branch. I added you as a contributor to my repository, so we can then work together on the same branch (rm4pro) in my repo. Focus is on getting the errors / warning / style issues resolved so we can merge it with the pending PR.

Hey @AntonJansen ,

I checked out your code and indeed some supressWarnings does the trick for the compiler warnings.

got rid of these in one of my last commits

There are also some style warnings we need to take care off.

I looked at this, but there is something wrong with my summary file for errors. Not only it appears not to change, it also appears inaccurate. for example, one of the warnings i get is:

checkstyle	2	98	style	MarkdownCheck	The line after code formatting section must be empty.

in README.md
I am pretty sure i solved that already, but the error still shows up. That happened with other errors too, so i could not advance much on this.

For the Mockito test INFO warnings, I have no idea :frowning:

Me neither, but i browsed around the forum, and it looks like we should be ok ignoring them.

You create a pull request to me and I merge it with my rm4pro branch.

Done.

I added you as a contributor to my repository,

I accepted the invite.

I would recommend that we put ourselves a deadline of end of this week to resolve these. We can also shoot a message in the PR letting them know we are mergint this this week. that way they do not waste time reviewing. Do you happen to know when does the window for 4.2 close?

Thanks!

Never mind Re: style warnings. The static analysis should be clean now. I closed my previous PR and opened a new one.

I have merged the PR. I will not have time today to work on this. I will spend some time tomorrow evening / the weekend. Hopefully, we can then merge it with the outstanding PR. I have already made comment about this change coming, so the reviewers of the PR should not be surprised.

By the way, I just run the merged code. I noticed you managed to get rid off the mockito info warnings. What was the cause? (just for my interest)

I got rid of them using supresswarnings. After spending a fair bit of time looking into it, i think the checking mechanism is kind of broken. All the functions are declared as null-able, so i could not find a reason why the compiler would throw a warning there.

I will try to finish my testing today. There is a couple of things that i would also like to get to, depending on time, before Friday:

-Currently, there are 2 sets of functions to store/modify/lookup/delete code, one for IR and one for RF. They are identical except for the storage they write to, so i will try to consolidate them, so there less duplicated code to maintain.

-It would be great if after any successful modification of the storage, the parameter that holds the command label gets erased, as to make sure no unwanted overwrites happen. Not even sure if that can be done though.

-A bit of a nitpick. i do not like the “Check and save” name used to add a code…would it be better to just say “Save new code”?

-I own 1 RM3, and 2 different types of RM4. I noticed that for the RM3 and one of the RM4, the process to add a thing is different ( for one, the interface only asks for a thing name, for the other one, there is a couple of options present to add as thing, etc etc). I will try and see if i canmake the behaviour the same for all of them.

-I notice the handler functions used to learn codes are quite different between RF and IR. They even have different names. i would your input to understand if we should consolidate names and if there is any chance they can be consolidated.

-I have not had time to look at dynamic channels. But i still think that can come after the binding is merged.

Thanks!

Thanks @AntonJansen for the binding!
I’m trying to connect a RM4mini, managed to see it in the things and have four channels. Learning of the code, gave me the code in the openhab.log, but now I’m stuck. I’m not able to switch on the aircon.
This is my item:

Switch Air_cond_on "Aircon ON 25°C" { channel="broadlink:rm4mini:2747c1ce9c:Aircon_ON" }

this the map file:

"Aircon_ON": {
    "value": "Aircon_ON=2600bc016d390d0f0d2b0d0f0e0f0c0f0d0f0d0f0d0f0d0f0d0f0e0f0d0f0c0f0d2b0d0f0d0f0d0f0d0f0d100c0f0d0f0d2b0d2b0d2b0c0f0d0f0d2b0d0f0d100d0f0d0f0c0f0d0f0d0f0d0f0d100d0f0d0f0d0f0c0f0d0f0d0f0d0f0d100d0f0d0f0d0f0c0f0d0f0d0f0d0f0d100d0f0d0f0d0e0d0f0d0f0d2b0d2b0d100d0e0d0f0d0f0d0f0d0001416d390d0f0d2a0d100c100d0f0d0f0d0f0d0f0c100d0f0c100d0f0d0f0d2b0d0f0d0f0d0e0d100c100c100d0f0d2b0d2a0d2b0d0f0d0f0d2b0d0f0d0f0c100d0f0d0f0d0f0d0f0d0f0d100c0f0c100d0f0d0f0d0f0d0f0d0f0d2b0c100c100c2c0d0f0d0f0d2a0d0f0d0f0c2c0d2b0c100d0f0d0f0d0f0d0f0d0f0d0f0d0f0d100b2c0c2c0d2b0d2b0c2b0d0f0d2b0d0f0c2c0c2b0d0f0d2b0d2b0d100b100d0f0d0f0d0f0d0f0d0f0d100c100b100d0f0d0f0d0f0d2b0d2b0c2b0d0f0d0f0d100c100d0f0c100c0f0c100d0f0d2b0d2b0d2b0c100d0f0c100c100d0f0d0f0c100b110c100d0f0d0f0d0f0c100d0f0d0e0d100c2c0c100d0f0d0f0c100d0e0c100d2c0c100c100c100d0f0d0e0c100d100c100c100c100c100d0f0c100c100c100c100c2c0c2c0d0e0c2c0d0f0d2b0d0f0d100c000d05"
}

What could be wrong?
Thanks!
Tom

P.S. through the app aircon is working fine

Hi,

Which version of the binding are you using?

Hi!
org.openhab.binding.broadlink-4.2.0-SNAPSHOT.jar

In the event file I see the log, but Rm4 mini doesn’t receive it (no white light).
Temp and humidity are working fine!
Thanks