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

[panasonicbdp] Initial contribution #16122

Merged
merged 14 commits into from
Jan 1, 2024
Merged

[panasonicbdp] Initial contribution #16122

merged 14 commits into from
Jan 1, 2024

Conversation

mlobstein
Copy link
Contributor

@mlobstein mlobstein commented Dec 27, 2023 •
edited
Loading

New binding to support Panasonic DP-UB420/424, DP-UB820/824 & DP-UB9000/9004 UHD Blu-ray players.
Also supports some older models including the DMP-BDT110, DMP-BDT210, DMP-BDT310, DMP-BDT120, DMP-BDT220, DMP-BDT320, DMP-BBT01 & DMP-BDT500.

Test build:

https://github.com/mlobstein/openhab-addons/releases/download/1.0/org.openhab.binding.panasonicbdp-4.2.0-SNAPSHOT.jar

@mlobstein
Panasonic Blu-ray initial
3713f76 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein
Merge branch 'main' into panabr
88d80c3
@mlobstein
update pom to 4.2.0-snapshot
fecff26 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein mlobstein added the new binding If someone has started to work on a binding. For a new binding PR. label Dec 27, 2023
@mlobstein mlobstein requested a review from a team as a code owner December 27, 2023 01:43
lsiepel
lsiepel requested changes Dec 30, 2023
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel 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 the contribution. Looks very solid!

Left some very minor comments.
code, i18n, javadoc, sat all look good

bundles/org.openhab.binding.panasonicbr/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbr/README.md Outdated Show resolved Hide resolved
...sonicbr/src/main/java/org/openhab/binding/panasonicbr/internal/PanaBlurayHandlerFactory.java Outdated Show resolved Hide resolved
@jlaur
Copy link
Contributor

jlaur commented Dec 30, 2023

@mlobstein - thanks for the contribution! One initial, potentially annoying question: What does "br" stand for? 🙂

@lolodomo
Copy link
Contributor

br => BluRay

@jlaur
Copy link
Contributor

jlaur commented Dec 30, 2023 •
edited
Loading

br => BluRay

That's what I'm afraid of, since BR is not a commonly used abbreviation for Blu-ray. BD is the official abbreviation ("Blu-ray Disc"). Also, BDP is commonly used for Blu-ray Disc Player, so in the context of this PR, "panasonicbdp" might be more appropriate. Hence the reference to my comment as "potentially annoying", because that would mean renaming everything.

@mlobstein
directory
7c8190b 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein
search and replace
51e4944 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein
rename package
4242ae1 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein
review changes
aed9827 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein
update pom.xml
d7f655c 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
lsiepel
lsiepel approved these changes Dec 30, 2023
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

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

LGTM

@mlobstein mlobstein changed the title [panasonicbr] Initial contribution [panasonicbdp] Initial contribution Dec 30, 2023
@lolodomo
Copy link
Contributor

This new binding is small, I will have a second look very soon.

jlaur
jlaur requested changes Dec 31, 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.

This new binding is small, I will have a second look very soon.

@lolodomo - 👍

I just now had a very speedy look out of curiosity, and as always the code looks very clean and nice, @mlobstein. 👍 I have posted some initial findings from scrolling through the files. 😉

...icbdp/src/main/java/org/openhab/binding/panasonicbdp/internal/handler/PanaBlurayHandler.java Outdated Show resolved Hide resolved
...icbdp/src/main/java/org/openhab/binding/panasonicbdp/internal/handler/PanaBlurayHandler.java Outdated Show resolved Hide resolved
...icbdp/src/main/java/org/openhab/binding/panasonicbdp/internal/handler/PanaBlurayHandler.java Outdated Show resolved Hide resolved
...icbdp/src/main/java/org/openhab/binding/panasonicbdp/internal/handler/PanaBlurayHandler.java Outdated Show resolved Hide resolved
...icbdp/src/main/java/org/openhab/binding/panasonicbdp/internal/handler/PanaBlurayHandler.java Outdated Show resolved Hide resolved
...icbdp/src/main/java/org/openhab/binding/panasonicbdp/internal/handler/PanaBlurayHandler.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/src/main/resources/OH-INF/thing/bdplayer.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/src/main/resources/OH-INF/thing/bdplayer.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/src/main/resources/OH-INF/thing/bdplayer.xml Outdated Show resolved Hide resolved
@lolodomo
Copy link
Contributor

So no need for me here, nice :)
You have the lead on this one @jlaur

@jlaur
Copy link
Contributor

jlaur commented Dec 31, 2023

So no need for me here, nice :)

Did not mean to steal this goodie from you, only had a quick glimpse out of curiosity, so please go ahead as planned. 🙂

@lolodomo
Copy link
Contributor

Did not mean to steal this goodie from you, only had a quick glimpse out of curiosity, so please go ahead as planned. 🙂

I have not yet started. Considering how many PRs are still opened, it would be more efficient if we are not doing the same thing but rather "work" on different PRs.

jlaur
jlaur requested changes Dec 31, 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.

Thanks for your contribution! I did a quick review with only a few findings, since the code already is in a good shape.

bundles/org.openhab.binding.panasonicbdp/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/README.md Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/README.md Outdated Show resolved Hide resolved
...icbdp/src/main/java/org/openhab/binding/panasonicbdp/internal/handler/PanaBlurayHandler.java Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/src/main/resources/OH-INF/thing/bdplayer.xml Outdated Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/src/main/resources/OH-INF/thing/uhdplayer.xml Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/src/main/resources/OH-INF/thing/bdplayer.xml Show resolved Hide resolved
bundles/org.openhab.binding.panasonicbdp/src/main/resources/OH-INF/thing/bdplayer.xml Show resolved Hide resolved
@mlobstein
review changes
66fa8e9 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein
update uhd_button and missing icons
b6d8db2 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein
add readme link
46cfd1c 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein
correct thing status updates
82901e1 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
@mlobstein
add missed status change and update header to 2024
eaa7e1c 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
jlaur
jlaur approved these changes Jan 1, 2024
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.

LGTM. Nice to know this binding is available as an alternative when my beloved Oppo UDP-203 dies one day. 🙂

bundles/org.openhab.binding.panasonicbdp/README.md Show resolved Hide resolved
@jlaur
Update bundles/org.openhab.binding.panasonicbdp/README.md
10aa00f 
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur merged commit db64df1 into openhab:main Jan 1, 2024
2 of 3 checks passed
@jlaur jlaur added this to the 4.2 milestone Jan 1, 2024
@mlobstein mlobstein deleted the panabr branch January 1, 2024 20:14
@mlobstein
Copy link
Contributor Author

LGTM. Nice to know this binding is available as an alternative when my beloved Oppo UDP-203 dies one day. 🙂

Thanks, Also good to know that someone else is using the oppo binding with a 203 since I only have a 103.

@mlobstein mlobstein mentioned this pull request Jan 2, 2024
Merged
Cybso pushed a commit to Cybso/openhab-addons that referenced this pull request Jan 5, 2024
@mlobstein @Cybso
[panasonicbdp] Initial contribution (openhab#16122)
27a1106 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@mlobstein @austvik
[panasonicbdp] Initial contribution (openhab#16122)
1b118fb 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
@mlobstein @joni1993
[panasonicbdp] Initial contribution (openhab#16122)
5e06318 
Signed-off-by: Michael Lobstein <michael.lobstein@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur approved these changes

@lsiepel lsiepel lsiepel approved these changes

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.2
Development

Successfully merging this pull request may close these issues.
4 participants
@mlobstein @jlaur @lolodomo @lsiepel