mob41 / broadlink-java-api

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

Android changes #23

Open psiniemi opened 4 years ago

psiniemi commented 4 years ago

To run on older android devices, I needed to remove references to Byte.toUnsignedInt and newer javas also don't ship with javax.xml anymore, so I also removed javax.xml.bind.DatatypeConverter which was only used for bytes -> hex conversion and replaced with a HexUtil class that can do the conversion both ways.

mob41 commented 4 years ago

Thanks for the modification. Would it be better to change the static import directly as import com.github.mob41.blapi.HexUtil and use HexUtil.byte2hex? Or is it specially used?

mob41 commented 4 years ago

Your pull request has a build error, which is caused by invalid maven pom.xml configuration: https://travis-ci.org/mob41/broadlink-java-api/builds/570753996

Removing the changes or fixing the invalid characters can help. Btw, what is the source configuration is for?

flo-02-mu commented 4 years ago

Is there any chance of getting this PR included? The ability to use the library with java 11 would be super nice. I added a PR onto the PR to fix the pom-file (https://github.com/psiniemi/broadlink-java-api/pulls) but no feedback so far.

mob41 commented 4 years ago

Unless author accepts your PR or makes changes to his code, I couldn’t merge two branches together cause it could break changes.

psiniemi commented 4 years ago

Sorry, completely forgot about this and somehow missed the notifications until now. Will look into the comments this weekend.

codecov[bot] commented 4 years ago

Codecov Report

Merging #23 into master will not change coverage. The diff coverage is 0%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #23   +/-   ##
=====================================
  Coverage       0%     0%           
=====================================
  Files          42     43    +1     
  Lines        1013   1024   +11     
  Branches      107     92   -15     
=====================================
- Misses       1013   1024   +11
Impacted Files Coverage Δ
...rc/main/java/com/github/mob41/blapi/SP2Device.java 0% <0%> (ø) :arrow_up:
src/main/java/com/github/mob41/blapi/A1Device.java 0% <0%> (ø) :arrow_up:
...ub/mob41/blapi/pkt/cmd/hysen/BaseHysenCommand.java 0% <0%> (ø) :arrow_up:
src/main/java/com/github/mob41/blapi/HexUtil.java 0% <0%> (ø)
...rc/main/java/com/github/mob41/blapi/SP1Device.java 0% <0%> (ø) :arrow_up:
src/main/java/com/github/mob41/blapi/BLDevice.java 0% <0%> (ø) :arrow_up:
...rc/main/java/com/github/mob41/blapi/MP1Device.java 0% <0%> (ø) :arrow_up:
...rc/main/java/com/github/mob41/blapi/RM2Device.java 0% <0%> (ø) :arrow_up:
.../github/mob41/blapi/dev/hysen/BaseHysenDevice.java 0% <0%> (ø) :arrow_up:
...ain/java/com/github/mob41/blapi/pkt/CmdPacket.java 0% <0%> (ø) :arrow_up:
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b5cc9c5...66c3f60. Read the comment docs.

psiniemi commented 4 years ago

Hi, static imports are pretty much a question of taste. The only downside I see is that if there are a lot of them, it can make following the code quite confusing. I like to use them as it makes the code shorter and as long as the name of the function name says what it does, I find it easier to follow. But since it appears our tastes differ, I removed the static import.