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

[pushover] Migration of Pushover OH1 action to OH3 binding #8586

Merged
merged 5 commits into from
Nov 29, 2020

Conversation

cweitkamp
Copy link
Contributor

  • Migration of Pushover action to OH3 binding

Replaces #6798

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

@cweitkamp cweitkamp added new binding If someone has started to work on a binding. For a new binding PR. work in progress A PR that is not yet ready to be merged oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2 labels Sep 26, 2020
@cweitkamp cweitkamp requested a review from a team as a code owner September 26, 2020 18:27
@wborn
Copy link
Member

wborn commented Oct 25, 2020

I finally gave it a test and it works! :-)
Is it still WIP because you want to make it possible to construct messages using all the other parameters?

@cweitkamp
Copy link
Contributor Author

cweitkamp commented Oct 26, 2020

@wborn Sounds good. I am using this port in my OH3 setup for several weeks and I did not see any error.

I think the list of available methods is finished. Are there any methods you are missing? If so please tell me and I will add it.

From my POV only the README is missing. I will try to finish it soon.

The absence of the PushoverBuilder is a loss for this add-on. But your recent community post about external Java classes in rules lets me think about exposing the new builder implementation and add some new actions to send a message by passing a builder. I will give it a try in the upcoming days. But this should not prevent this PR from finalization.

Another idea I have in my mind is to send the content on an ImageItem (RawType) attached to a Pushover message directly by passing it to a new action. But that is not urgent too.

@wborn
Copy link
Member

wborn commented Oct 26, 2020

It would already be nice to have the functionality of this PR available in OH3! The only additional parameter I use is the URL.

When the builder is moved to a non-internal package I think it can also be used in rule engines which would again provide a nice way to build a message with any parameter.

@cweitkamp
Copy link
Contributor Author

I added a sendURLMessage(message, @Nullable title, url, @Nullable urlTitle) action.

@matulekpl
Copy link

can someone please give me a .jar file for this?:)

@wborn
Copy link
Member

wborn commented Nov 18, 2020

Jenkins made one for this PR here org.openhab.binding.pushover-3.0.0-SNAPSHOT.jar.

@randye007
Copy link

randye007 commented Nov 18, 2020

Thanks for the jar file. I've successfully installed it and created a thing which is now online.
When I create the rule, I select the Type of Action (send an HTML message) and the Configuration (Pushover Account Thing).
However, I don't see a field for the actual message to send due to the action limitations outlined above. Can someone please tell me how I can configure the message content? Thx.

@matulekpl
Copy link

Thanks for the jar file. I've successfully installed it and created a thing which is now online.
When I create the rule, I select the Type of Action (send an HTML message) and the Configuration (Pushover Account Thing).
However, I don't see a field for the actual message to send due to the action limitations outlined above. Can someone please tell me how I can configure the message content? Thx.

How You installed it? I got it in addons folder but nothing happened...

@matulekpl
Copy link

Jenkins made one for this PR here org.openhab.binding.pushover-3.0.0-SNAPSHOT.jar.

weird thing - I added this file to /etc/openhab/addons/ and nothing happens.
Even after reboot of whole RPI4 there is no such binding on binding:list

@randye007
Copy link

How You installed it?

Just create the thing and you will see the option for Pushover.
I also don't see the binding through the UI or karaf console. Not sure why. Maybe it's because it's an action.

@matulekpl
Copy link

How You installed it?

Just create the thing and you will see the option for Pushover.
I also don't see the binding through the UI or karaf console. Not sure why. Maybe it's because it's an action.

I dont see anything with word "pushover" on creating Thing?

@wborn
Copy link
Member

wborn commented Nov 20, 2020

It works fine for me with the JAR:

pushover

@matulekpl
Copy link

It works fine for me with the JAR:

pushover

That is weird that it doesn’t work at mine.

To what folder did You put this file?

@randye007
Copy link

@matulekpl - on my rPi4 running OH3 M2, it's in the directory /usr/share/openhab/addons. Also, check the permissions on your jar file and make sure to assign openhab:openhab to the owner:group.
I was able to get everything working. You cannot currently use the UI to enter pushover message contents. You need to use scripts.

@matulekpl
Copy link

matulekpl commented Nov 20, 2020

@matulekpl - on my rPi4 running OH3 M2, it's in the directory /usr/share/openhab/addons. Also, check the permissions on your jar file and make sure to assign openhab:openhab to the owner:group.
I was able to get everything working. You cannot currently use the UI to enter pushover message contents. You need to use scripts.

Thanks for that. Now it is working.

And now another problem;)
Earlier on OH2 in rules it worked like

sendPushoverMessage(pushoverBuilder("test message"))
And now??

Edit:
Ok there was no question:D

val actions = getActions("pushover", "pushover:pushover-account:account")
// send HTML message
actions.sendHtmlMessage("Hello <font color='green'>World</font>!", "openHAB")

@randye007
Copy link

randye007 commented Nov 20, 2020

Try this:

val actions = getActions("pushover", "pushover:pushover-account:xxxxxxxxx")
actions.sendMessage("Hello World", "openHAB")

It's based on the example given in the README file.

sendAttachmentMessage()
sendHtmlMessage()
sendURLMessage()

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cweitkamp cweitkamp removed the work in progress A PR that is not yet ready to be merged label Nov 21, 2020
@cweitkamp cweitkamp changed the title [WIP][pushover] Migration of Pushover OH1 action to OH3 binding [pushover] Migration of Pushover OH1 action to OH3 binding Nov 21, 2020
@cweitkamp
Copy link
Contributor Author

I added a final version for the README.md and thus do not consider this as WIP any longer.

I did not yet test exposing the message builder.

Another thing which came to my mind was the general structure of Things. It is possible to define multiple users or groups for broadcasting messages. Maybe a Bridge -> Thing structure has to be considered.

@cweitkamp cweitkamp requested a review from wborn November 21, 2020 15:21
Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Copy link
Contributor

@cpmeister cpmeister left a comment

Choose a reason for hiding this comment

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

You also need to try and address the build warnings.

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Copy link
Contributor

@cpmeister cpmeister left a comment

Choose a reason for hiding this comment

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

Other than these last few comments. LGTM

Copy link
Contributor

@cpmeister cpmeister left a comment

Choose a reason for hiding this comment

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

Spoke too soon. Final comment.

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
@cpmeister cpmeister added the cre Coordinated Review Effort label Nov 27, 2020
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 a lot for the migration 👍 The code looks great too so let's merge it! :-)

@wborn wborn merged commit 6e0caca into openhab:main Nov 29, 2020
@wborn wborn added this to the 3.0.0.M4 milestone Nov 29, 2020
@wborn wborn removed the cre Coordinated Review Effort label Nov 29, 2020
@cweitkamp cweitkamp deleted the feature-pushover-binding branch November 29, 2020 11:45
@kaikreuzer
Copy link
Member

@cweitkamp This part of the README looks a bit broken: https://next.openhab.org/addons/bindings/pushover/#thing-actions

@cweitkamp
Copy link
Contributor Author

Yes, looks weird. I submitted #9184 to fix it.

@felixpause
Copy link

@cweitkamp, thanks for the migration. Works fine with the latest snapshot for me.
Just one proposal to make it easier to use with the new UI. Could you add the binding to the set of actions available when creating a rule? habot for example is listed there.
image

@cweitkamp
Copy link
Contributor Author

We hide all binding actions in UIs until a solution for openhab/openhab-core#1745 has been implemented.

nowaterman pushed a commit to nowaterman/openhab-addons that referenced this pull request Jan 19, 2021
boehan pushed a commit to boehan/openhab-addons that referenced this pull request Apr 12, 2021
)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
)

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new binding If someone has started to work on a binding. For a new binding PR. oh1 migration Relates to migrating an openHAB 1 addon to openHAB 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants