Skip to content

Conversation

@psiniemi
Copy link

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.

@a1aw
Copy link
Owner

a1aw commented Aug 12, 2019

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?

@a1aw
Copy link
Owner

a1aw commented Aug 12, 2019

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
Copy link

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.

@a1aw
Copy link
Owner

a1aw commented Dec 5, 2019

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

Remove incorrect characters from pom file
@psiniemi
Copy link
Author

psiniemi commented Dec 6, 2019

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

@codecov
Copy link

codecov bot commented Dec 7, 2019

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%> (ø) ⬆️
src/main/java/com/github/mob41/blapi/A1Device.java 0% <0%> (ø) ⬆️
...ub/mob41/blapi/pkt/cmd/hysen/BaseHysenCommand.java 0% <0%> (ø) ⬆️
src/main/java/com/github/mob41/blapi/HexUtil.java 0% <0%> (ø)
...rc/main/java/com/github/mob41/blapi/SP1Device.java 0% <0%> (ø) ⬆️
src/main/java/com/github/mob41/blapi/BLDevice.java 0% <0%> (ø) ⬆️
...rc/main/java/com/github/mob41/blapi/MP1Device.java 0% <0%> (ø) ⬆️
...rc/main/java/com/github/mob41/blapi/RM2Device.java 0% <0%> (ø) ⬆️
.../github/mob41/blapi/dev/hysen/BaseHysenDevice.java 0% <0%> (ø) ⬆️
...ain/java/com/github/mob41/blapi/pkt/CmdPacket.java 0% <0%> (ø) ⬆️
... 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
Copy link
Author

psiniemi commented Dec 9, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants