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

Added plugin-related functions to the Java client #1979

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

moritzvieli
Copy link

  • Add OlaClient.reloadPlugins()
  • Add OlaClient.getPluginState()
  • Add OlaClient.setPluginState()
  • Changed evaluation of PROTOBUF_VERSION, because pkg-config adds trailing ".0" to the version, which is not compatible with the available Java dependencies

The last change needs to be checked carefully. I tried building it on a Mac and it failed with the pkg-config version, because of the trailing ".0". Using "protoc --version" works, but might introduce other issues.

- Add OlaClient.reloadPlugins()
- Add OlaClient.getPluginState()
- Add OlaClient.setPluginState()
- Changed evaluation of PROTOBUF_VERSION, because pkg-config adds trailing ".0" to the version, which is not compatible with the available Java dependencies
Copy link
Member

@peternewman peternewman left a comment

Choose a reason for hiding this comment

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

Thanks @moritzvieli .

I don't know whether you want to split this in two, as apart from some minor styling, the client changes are ready to go, whereas the Protobuf Java versioning may need a bit more testing and thought.

It would also be good to add some tests too please:
https://github.com/OpenLightingProject/ola/blob/master/java/src/test/java/ola/OlaClientTest.java

java/pom.xml Outdated Show resolved Hide resolved
java/ChangeLog Outdated Show resolved Hide resolved
Comment on lines -39 to +40
<source>1.6</source>
<target>1.6</target>
<source>1.7</source>
<target>1.7</target>
Copy link
Member

Choose a reason for hiding this comment

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

Is this just because 1.6 is out of support?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. I was unable to compile this Java version with my locally installed Java version 21. 1.7 is still outdated, but a little bit less worse imo.

java/src/main/java/ola/OlaClient.java Show resolved Hide resolved
java/src/main/java/ola/OlaClient.java Outdated Show resolved Hide resolved
java/src/main/java/ola/OlaClient.java Outdated Show resolved Hide resolved
java/src/main/java/ola/OlaClient.java Outdated Show resolved Hide resolved
java/src/main/java/ola/OlaClient.java Outdated Show resolved Hide resolved
java/src/main/java/ola/OlaClient.java Outdated Show resolved Hide resolved
config/ola.m4 Outdated Show resolved Hide resolved
@moritzvieli
Copy link
Author

@peternewman I incorporated all changes. The conversation about the Java source version is not yet resolved. Please let me know, if I missed something. Thanks a lot!

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.

2 participants