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

[pushbullet] Add link and file push type support #17472

Merged
merged 6 commits into from
Sep 28, 2024
Merged

[pushbullet] Add link and file push type support #17472

merged 6 commits into from
Sep 28, 2024

Conversation

jsetton
Copy link
Contributor

@jsetton jsetton commented Sep 24, 2024 •
edited
Loading

This change adds link and file push type support as defined in the Pushbullet API documentation.

This includes the ability to send messages attaching the content of an image URL, a local file or an Image item state.

@jsetton jsetton requested a review from hakan42 as a code owner September 24, 2024 06:40
@lsiepel lsiepel added the enhancement An enhancement or new feature for an existing add-on label Sep 24, 2024
@jsetton jsetton force-pushed the pushbullet branch 6 times, most recently from 8be23ab to 739ee5b Compare September 25, 2024 01:41
@jsetton
[pushbullet] Add link and file push type support
65a4ded 
Signed-off-by: jsetton <jeremy.setton@gmail.com>
lsiepel
lsiepel requested changes Sep 26, 2024
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.

In general it looks very good, nice additions! Left some comments to look at.

bundles/org.openhab.binding.pushbullet/README.md Outdated Show resolved Hide resolved
...g.pushbullet/src/main/java/org/openhab/binding/pushbullet/internal/PushbulletHttpClient.java Outdated Show resolved Hide resolved
...g.pushbullet/src/main/java/org/openhab/binding/pushbullet/internal/PushbulletHttpClient.java Show resolved Hide resolved
...hbullet/src/main/java/org/openhab/binding/pushbullet/internal/handler/PushbulletHandler.java Outdated Show resolved Hide resolved
...hbullet/src/main/java/org/openhab/binding/pushbullet/internal/handler/PushbulletHandler.java Show resolved Hide resolved
...hbullet/src/main/java/org/openhab/binding/pushbullet/internal/handler/PushbulletHandler.java Outdated Show resolved Hide resolved
@jsetton
[pushbullet] Code review changes
24680bc 
Signed-off-by: jsetton <jeremy.setton@gmail.com>
@lsiepel lsiepel added the rebuild Triggers Jenkins PR build label Sep 28, 2024
lsiepel
lsiepel requested changes Sep 28, 2024
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.

Last comment, otherwise LGTM. Thanks for addressing it quickly!

...hbullet/src/main/java/org/openhab/binding/pushbullet/internal/handler/PushbulletHandler.java Outdated Show resolved Hide resolved
@lsiepel lsiepel removed the rebuild Triggers Jenkins PR build label Sep 28, 2024
@jsetton
[pushbullet] Code review additional changes
59268dc 
Signed-off-by: jsetton <jeremy.setton@gmail.com>
@jsetton
[pushbullet] Add api too many requests warning
6255106 
Signed-off-by: jsetton <jeremy.setton@gmail.com>
@jsetton
Copy link
Contributor Author

jsetton commented Sep 28, 2024 •
edited
Loading

I was wondering if the channel configuration should be removed. There is no implementation in the binding supporting them and is confusing. Same goes with the items and sitemap examples at the end of the documentation.

As far as the thing configuration, I think that the apiUrlBase parameter should be hardcoded. There is no way to run your own Pushbullet server as far as I know. And the name parameter isn't used as well.

@lsiepel
Copy link
Contributor

lsiepel commented Sep 28, 2024

I was wondering if the channel configuration should be removed. There is no implementation in the binding supporting them and is confusing. Same goes with the items and sitemap examples at the end of the documentation.

As far as the thing configuration, I think that the apiUrlBase parameter should be hardcoded. There is no way to run your own Pushbullet server as far as I know. And the name parameter isn't used as well.

I prefer to seperate PR's for these kind of clean-up.

@jsetton
[pushbullet] Add missing i18n thing and channel types entries
df90c8f 
Signed-off-by: jsetton <jeremy.setton@gmail.com>
@jsetton
[pushbullet] Restore default label and description resource entries
2ce1eca 
Signed-off-by: jsetton <jeremy.setton@gmail.com>
lsiepel
lsiepel approved these changes Sep 28, 2024
View reviewed changes
Copy link
Contributor

@lsiepel lsiepel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, LGTM.

Do you need some time to finish tests and validate no regressions happened due to all the recent commits? I'll merge otherwise.

@jsetton
Copy link
Contributor Author

jsetton commented Sep 28, 2024 •
edited
Loading

Do you need some time to finish tests and validate no regressions happened due to all the recent commits?

I already tested that I could push a note, link and file successfully with the last changes I made. So you can merge this PR. Thanks.

@lsiepel lsiepel merged commit c889745 into openhab:main Sep 28, 2024
5 checks passed
@lsiepel lsiepel added this to the 4.3 milestone Sep 28, 2024
@jsetton jsetton deleted the pushbullet branch September 28, 2024 21:17
@jsetton
Copy link
Contributor Author

jsetton commented Sep 28, 2024

I prefer to seperate PR's for these kind of clean-up.

#17482

joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
@jsetton @joni1993
[pushbullet] Add link and file push type support (openhab#17472)
7e64f2e 
* [pushbullet] Add link and file push type support

Signed-off-by: jsetton <jeremy.setton@gmail.com>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
@jsetton @matchews
[pushbullet] Add link and file push type support (openhab#17472)
73383fc 
* [pushbullet] Add link and file push type support

Signed-off-by: jsetton <jeremy.setton@gmail.com>
cipianpascu pushed a commit to cipianpascu/openhab-addons that referenced this pull request Jan 2, 2025
@jsetton
[pushbullet] Add link and file push type support (openhab#17472)
72eb777 
* [pushbullet] Add link and file push type support

Signed-off-by: jsetton <jeremy.setton@gmail.com>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsiepel lsiepel lsiepel approved these changes

@hakan42 hakan42 Awaiting requested review from hakan42 hakan42 is a code owner

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
2 participants
@jsetton @lsiepel