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.

Sign up for GitHub

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

Jump to bottom

[volumio] Initial contribution #14525

Merged
merged 29 commits into from
Jul 7, 2023
Merged

[volumio] Initial contribution #14525

merged 29 commits into from
Jul 7, 2023

Conversation

miloit
Copy link
Contributor

@miloit miloit commented Mar 3, 2023

This is a new plugin for a volumio device

@miloit miloit requested a review from a team as a code owner March 3, 2023 14:29
@miloit miloit mentioned this pull request Mar 3, 2023
Closed
@lsiepel lsiepel changed the title [volumio] Initial commit to official addon repo [volumio] Initial contribution Mar 3, 2023
@lsiepel
Copy link
Contributor

lsiepel commented Mar 3, 2023

@miloit Thanks for your contribution. I can do a basic review, but before i do there are some things to fix:
The build is failing, there are three commits without DCO and i see some placeholders in the readme.
I also wonder where the different @author tags come from as this seems to be a initial contribution ? Maybe you wnat to update the first post with some details too.

@miloit
Copy link
Contributor Author

miloit commented Mar 3, 2023 •
edited
Loading

Hello @lsiepel it's the first contribution of this addon to the office openhab repo. The binding come from https://github.com/stargazer74/org.openhab.binding.volumio2/releases/tag/3.4.0

But I improved it to make it compatible to oh3.4 and also removed/improved some failures....the readme and so on I will do in the coming days. I am not a developer so I need to do everything step by step learning my apologies for this...

It would be great if you can do a quick review so that I can see what else needs to be improved corrected :)

@miloit miloit force-pushed the main branch 5 times, most recently from d82dbc4 to ae58cdc Compare March 4, 2023 10:30
@miloit
Copy link
Contributor Author

miloit commented Mar 5, 2023 •
edited
Loading

@openhab/add-ons-maintainers @lsiepel Binding is now ready for review

@jlaur jlaur added the new binding If someone has started to work on a binding. For a new binding PR. label Mar 7, 2023
@wborn wborn added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Mar 10, 2023
@wborn
Copy link
Member

wborn commented Mar 10, 2023

Perhaps @stargazer74 and @patrickse can help reviewing and testing as they also contributed code parts?

wborn
wborn requested changes Mar 10, 2023
View reviewed changes
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Thanks for creating the PR!

I didn't look at the code but saw these few issues already.

Please also:

.gitignore Outdated Show resolved Hide resolved
bundles/org.openhab.binding.volumio/src/main/resources/OH-INF/binding/binding.xml.bak Outdated Show resolved Hide resolved
bundles/org.openhab.binding.volumio/src/main/resources/OH-INF/binding/binding.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.volumio/src/main/resources/OH-INF/i18n/volumio_de_DE.properties Outdated Show resolved Hide resolved
bundles/org.openhab.binding.volumio/src/main/resources/OH-INF/i18n/volumio.properties Outdated Show resolved Hide resolved
miloit and others added 11 commits March 10, 2023 16:39
@miloit
initial commit to official addon repo
4bba60a 
initial commit to official addon repo

Signed-off-by: miloit <[email protected]>
@miloit
sign off
022283b 
Signed-off-by: Michael Loercher<[email protected]>
Signed-off-by: miloit <[email protected]>
@miloit
Signed-off-by: miloit <[email protected]>
2ad27bb
@miloit
Readme update
c02fe1d 
Signed-off-by: miloit <[email protected]>
@miloit
Improved code
2931287 
Signed-off-by: miloit <[email protected]>
@miloit
Introducing Nullable class annotation to all classes
5e4db31 
Signed-off-by: miloit <[email protected]>
@miloit
Update Readme file
c93231d 
Signed-off-by: miloit <[email protected]>
@miloit
Improved coding to avoid null pointer exceptions. Renaming thing to v…
0ff0525 
…olumio.

Signed-off-by: miloit <[email protected]>
@miloit @wborn
Update bundles/org.openhab.binding.volumio/pom.xml
d9dfb9a 
Co-authored-by: Wouter Born <[email protected]>
Signed-off-by: miloit <[email protected]>
@miloit @wborn
Update .gitignore
47a260e 
Co-authored-by: Wouter Born <[email protected]>
Signed-off-by: miloit <[email protected]>
@miloit
Signed-off-by: miloit <[email protected]>
eb7f327
@miloit miloit requested a review from wborn March 10, 2023 15:40
wborn
wborn requested changes Apr 15, 2023
View reviewed changes
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Please also update the binding to follow these recently established naming conventions:

https://next.openhab.org/docs/developer/guidelines.html#naming-convention

@miloit miloit requested a review from wborn April 24, 2023 17:33
@miloit
Copy link
Contributor Author

miloit commented Apr 28, 2023

@openhab/add-ons-maintainers anything happening here?

@jlaur
Copy link
Contributor

jlaur commented Jun 17, 2023

@wborn - in case this PR was forgotten - gentle ping. :)

jlaur
jlaur requested changes Jun 17, 2023
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

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

Spotted a few formatting issues.

bundles/org.openhab.binding.volumio/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.volumio/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.volumio/README.md Show resolved Hide resolved
bundles/org.openhab.binding.volumio/README.md Show resolved Hide resolved
@miloit
updtaed formatting
f339134 
Signed-off-by: miloit <[email protected]>
@miloit miloit requested a review from jlaur June 18, 2023 09:18
@openhab openhab deleted a comment from miloit Jun 24, 2023
wborn
wborn requested changes Jun 24, 2023
View reviewed changes
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Looks like some remarks still haven't been processed.

bundles/org.openhab.binding.volumio/src/main/resources/OH-INF/addon/addon.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.volumio/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved
@miloit miloit requested a review from wborn July 6, 2023 19:43
@miloit
Copy link
Contributor Author

miloit commented Jul 6, 2023

solved now

@openhab openhab deleted a comment from miloit Jul 7, 2023
wborn
wborn requested changes Jul 7, 2023
View reviewed changes
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comments! 👍 I went through the code one last time which resulted in a few more comments. 😉

bundles/org.openhab.binding.volumio/src/main/resources/OH-INF/i18n/volumio.properties Outdated Show resolved Hide resolved
...enhab.binding.volumio/src/main/java/org/openhab/binding/volumio/internal/VolumioHandler.java Outdated Show resolved Hide resolved
...enhab.binding.volumio/src/main/java/org/openhab/binding/volumio/internal/VolumioHandler.java Outdated Show resolved Hide resolved
...enhab.binding.volumio/src/main/java/org/openhab/binding/volumio/internal/VolumioHandler.java Outdated Show resolved Hide resolved
...enhab.binding.volumio/src/main/java/org/openhab/binding/volumio/internal/VolumioHandler.java Outdated Show resolved Hide resolved
....binding.volumio/src/main/java/org/openhab/binding/volumio/internal/mapping/VolumioData.java Outdated Show resolved Hide resolved
...enhab.binding.volumio/src/main/java/org/openhab/binding/volumio/internal/VolumioHandler.java Show resolved Hide resolved
...enhab.binding.volumio/src/main/java/org/openhab/binding/volumio/internal/VolumioHandler.java Outdated Show resolved Hide resolved
...enhab.binding.volumio/src/main/java/org/openhab/binding/volumio/internal/VolumioHandler.java Outdated Show resolved Hide resolved
...enhab.binding.volumio/src/main/java/org/openhab/binding/volumio/internal/VolumioHandler.java Outdated Show resolved Hide resolved
@miloit miloit requested a review from wborn July 7, 2023 09:08
@miloit miloit requested a review from wborn July 7, 2023 09:52
wborn
wborn approved these changes Jul 7, 2023
View reviewed changes
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Many thanks! LGTM now so let's merge it. 🚀

@wborn wborn merged commit de1eebd into openhab:main Jul 7, 2023
@wborn wborn added this to the 4.0 milestone Jul 7, 2023
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Jul 8, 2023
@miloit @markus7017
[volumio] Initial contribution (openhab#14525)
d26aeee 
Signed-off-by: Michael Loercher <[email protected]>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Aug 9, 2023
@miloit @matchews
[volumio] Initial contribution (openhab#14525)
13b95c2 
Signed-off-by: Michael Loercher <[email protected]>
Signed-off-by: Matt Myers <[email protected]>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@miloit @austvik
[volumio] Initial contribution (openhab#14525)
8b89cac 
Signed-off-by: Michael Loercher <[email protected]>
Signed-off-by: Jørgen Austvik <[email protected]>
@jlaur jlaur mentioned this pull request May 11, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wborn wborn wborn approved these changes

@hmerk hmerk Awaiting requested review from hmerk

@jlaur jlaur Awaiting requested review from jlaur

Assignees
No one assigned
Labels
new binding If someone has started to work on a binding. For a new binding PR.
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

[volumio] Add new binding to openhab
5 participants
@miloit @lsiepel @wborn @hmerk @jlaur