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.

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

[Broadlink] Initial contribution #16273

Merged
merged 226 commits into from
Oct 16, 2024
Merged

Conversation

AntonJansen
Copy link
Contributor

@AntonJansen AntonJansen commented Jan 13, 2024

Title

  • [Broadlink] A binding that can communicate with the Broadlink family of WiFi-addressable devices

Description

The binding support the following devices:

  • A1 environmental sensor
  • RMx Remote IR blasters
  • SPx WiFi-controlled power switches
  • MPx WiFi-controlled multi-outlet power strips

This pull request is an update of the stalled pull request #14582, updated for 4.2.x

The discussion for this pull request is:
https://community.openhab.org/t/broadlink-binding-4-1-0-4-2-0/154734

Signed-off-by: Anton Jansen mail@antonjansen.com

@AntonJansen AntonJansen requested a review from a team as a code owner January 13, 2024 18:58
@lsiepel lsiepel added the new binding If someone has started to work on a binding. For a new binding PR. label Jan 13, 2024
Signed-off-by: AntonJansen <gradius@fmf.nl>
…nes.

Signed-off-by: AntonJansen <gradius@fmf.nl>
…will work.

Signed-off-by: AntonJansen <gradius@fmf.nl>
Signed-off-by: AntonJansen <gradius@fmf.nl>
- a reference to https:// that should have been http:// for xsi:schemaLocation
- a double copyright reference
- an empty line that should not be there

Signed-off-by: AntonJansen <gradius@fmf.nl>
@openhab-bot
Copy link
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/16

@lsiepel lsiepel changed the title [Broadlink] Broadlink binding 4.2.x [Broadlink] Initial contribution Jan 14, 2024
- Add new device type to model mapper
- Fix bug with mac address being send in wrong order
- Added trace logging information to track messages

Signed-off-by: AntonJansen <gradius@fmf.nl>
Cleaned up the code.

Signed-off-by: AntonJansen <gradius@fmf.nl>
…tion.

Signed-off-by: AntonJansen <gradius@fmf.nl>
Signed-off-by: AntonJansen <gradius@fmf.nl>
Signed-off-by: AntonJansen <gradius@fmf.nl>
Signed-off-by: AntonJansen <gradius@fmf.nl>
Copy link
Contributor

@Skinah Skinah left a comment

Choose a reason for hiding this comment

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

Please go through all uses of the logger and fix.

Also after you have built the binding look in the target folder where the jar appears.
Folder called target\code-analysis
open the file called report.html
Then try and fix as many as you can whilst you wait for more review comments.

Thanks for taking on the binding.

AntonJansen and others added 11 commits January 27, 2024 13:56
…b/binding/broadlink/handler/BroadlinkA1Handler.java


Reducing trace / debug logging to confirm to logging policy.

Co-authored-by: Matthew Skinner <matt@pcmus.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/handler/BroadlinkBaseThingHandler.java


Reducing trace / debug logging to confirm to logging policy.

Co-authored-by: Matthew Skinner <matt@pcmus.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/handler/BroadlinkBaseThingHandler.java


Reducing trace / debug logging to confirm to logging policy.

Co-authored-by: Matthew Skinner <matt@pcmus.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/handler/BroadlinkBaseThingHandler.java


Reducing trace / debug logging to confirm to logging policy.

Co-authored-by: Matthew Skinner <matt@pcmus.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/handler/BroadlinkBaseThingHandler.java


Reducing trace / debug logging to confirm to logging policy.

Co-authored-by: Matthew Skinner <matt@pcmus.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/handler/BroadlinkBaseThingHandler.java


Reducing trace / debug logging to confirm to logging policy.

Co-authored-by: Matthew Skinner <matt@pcmus.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/handler/BroadlinkBaseThingHandler.java


Ensure actual message is included in logging

Co-authored-by: Matthew Skinner <matt@pcmus.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/handler/BroadlinkBaseThingHandler.java


Reducing trace / debug logging to confirm to logging policy.

Co-authored-by: Matthew Skinner <matt@pcmus.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
Signed-off-by: AntonJansen <gradius@fmf.nl>
Signed-off-by: AntonJansen <gradius@fmf.nl>
Signed-off-by: AntonJansen <gradius@fmf.nl>
@lsiepel
Copy link
Contributor

lsiepel commented Sep 19, 2024

I guess the commits where made through the Github web interface. When not properly configured, this causes the DCO to fail because of missing signoff. Unfortunately the suggestions had some whitespace issues so spotless is also complaining.

Hopefully you manage to rewrite the commit messages with a signoff and fix the spotless issue. I would also ask you to have a test run after all these change have been applied. It will not be the first and last time my suggestions lead to regression.

AntonJansen and others added 5 commits September 29, 2024 17:20
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…F/thing/rm-types.xml

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…F/thing/channels.xml

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…ing issues.

Signed-off-by: AntonJansen <gradius@fmf.nl>
@AntonJansen
Copy link
Contributor Author

AntonJansen commented Sep 29, 2024

@lsiepel I should have addressed all your comments now. The main thing left is the DCO, which is due to Ricardo Larranaga, which did not use the proper sign-off. I have been trying to reach him the last 6 months, but no luck thus far.

Signed-off-by: AntonJansen <gradius@fmf.nl>
@lsiepel
Copy link
Contributor

lsiepel commented Sep 29, 2024

@lsiepel I should have addressed all your comments now. The main thing left is the DCO, which is due to Ricardo Larranaga, which did not use the proper sign-off. I have been trying to reach him the last 6 months, but no luck thus far.

I’ll try to have another look next week.

Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

Finished reviewing the last files, also found some other improvements. I think this is the last iteration, a lot of work has been done. Thank you for adapting the improvements regarding the excessive reachability checks. Much better now.

AntonJansen and others added 11 commits October 14, 2024 19:55
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/internal/config/BroadlinkDeviceConfiguration.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/internal/config/BroadlinkDeviceConfiguration.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/internal/NetworkUtils.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
…b/binding/broadlink/internal/handler/BroadlinkBaseThingHandler.java

Co-authored-by: lsiepel <leosiepel@gmail.com>
Signed-off-by: Anton Jansen <AntonJansen@users.noreply.github.com>
Signed-off-by: AntonJansen <gradius@fmf.nl>
Signed-off-by: AntonJansen <gradius@fmf.nl>
Signed-off-by: AntonJansen <gradius@fmf.nl>
@AntonJansen
Copy link
Contributor Author

@lsiepel I resolved all the issues apart from one, for which I need a bit more input.

Signed-off-by: AntonJansen <gradius@fmf.nl>
@AntonJansen
Copy link
Contributor Author

@lsiepel all known issues should now have been resolved. Let me know what to do next.

@AntonJansen
Copy link
Contributor Author

@Skinah All your requested changes should have been completed. Please confirm, so we can wrap-up this pull request.

@lsiepel lsiepel merged commit e32fccb into openhab:main Oct 16, 2024
4 of 5 checks passed
@lsiepel lsiepel added this to the 4.3 milestone Oct 16, 2024
@lsiepel
Copy link
Contributor

lsiepel commented Oct 16, 2024

Thanks, now this is merged, you could add your binding's logo to the openHAB website. See https://next.openhab.org/docs/developer/addons/#add-your-add-on-s-logo-to-the-openhab-website-and-the-ui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

7 participants