mob41 / broadlink-java-api

A clean Broadlink API for Java
MIT License
22 stars 25 forks source link

Merge of two projects #2

Closed computerlyrik closed 7 years ago

computerlyrik commented 7 years ago

Hi! I merged the work of the three of us @mob41 , @bwssytems , @computerlyrik together.

This pull request contains the original features from PR at https://github.com/bwssytems/broadlink-java-api/pull/2

Additionally it fixes imports and description structure (small functional differences) between the forks.

@mob41 please have a look, if it fits for you and try out the test and @bwssytems please have a look at the copyright headers if this is okay for you

OpenHAB plugin will not be far anymore ... i still experience some undefined values from my A1 sensor. Fixes will follow.

mob41 commented 7 years ago

First of all, thank you for contributing in this library!

Just a question, should we do the log.isDebugEnabled() check before running log.debug(String msg)? like here https://github.com/mob41/broadlink-java-api/commit/64b94a9309f3cda19222f36ed356c2e5715c4c5b#diff-9799a5f1ebeab1099a7354cee22ad07fL60

Are there performance benefits if running isDebugEnabled() first?

mob41 commented 7 years ago

And I won't be available till next Monday. I will review and merge the pull request around 4-6 days later.

By the way, I really appreciate your work on this!

Another problem that is the copyright statement copied in each source code.

In your code about the copyright notice, you appended your name after mine Copyright (c) 2017 Anthony Law, ...

As I don't have much experience with putting license notice, I took a look with the Eclipse Copyright Fixer's format. It suggested to put a contributors (per commit) under the license text (above the **/.

Which way to specify your work to the source code do you prefer? I would prefer putting under the license text as it seems more clean.

bwssytems commented 7 years ago

That's Great! Did it test out ok? I do not have a broadlink rm pro and needed someone to test my changes.

computerlyrik commented 7 years ago
mob41 commented 7 years ago
[main] INFO com.github.mob41.blapi.DevicesTest - RMDevice get temperature: -79.3

I think there're still problems with RMDevice's temperature. Sometimes is correct, and sometimes incorrect. Whatever I will review all this night and merge this.

mob41 commented 7 years ago

Would you like me to let you be a co-maintainer?

computerlyrik commented 7 years ago

I do face some communication errors using my A1 Env sensor. Readings are never correct - and the Package (RAW UDP) are different to using the python library. I do not know in detail about the Protocol. I cannot decide if the DISCOVERY -> AUTHENTICATION -> QUERY Process runs correctly. How do we harden the communication Protocol?

About Maintenance: If this helps to prevent SPOFs - yes, i can help out as hot spare.

computerlyrik commented 7 years ago

I have a SP2 Device now - it is recognized by the junit-Test-method but it does not communicate correctly either.

mob41 commented 7 years ago

I have added you as a collaborator. But I would recommend you to do a pull request before push it.

bwssytems commented 7 years ago

What else can I do to help. Unfortunately, I don't have any Broadlink devices currently.

mob41 commented 7 years ago

I would recommend you to buy a RM Pro (with RF) which is pretty useful and cheap for simple home automation using the phone.

And... help with debugging the API :smile:

bwssytems commented 7 years ago

Of course, that is the answer! Yes, another piece of equipment to integrate :-)