这是indexloc提供的服务,不要输入任何密码
Skip to content
This repository was archived by the owner on May 17, 2021. It is now read-only.

Conversation

@bruestel
Copy link

Signed-off-by: Jonas Brüstel jonas@bruestel.net (github: bruestel)

Jonas Brüstel added 2 commits April 13, 2019 14:18
Addedd HTML and Monospace formatting option (https://pushover.net/api#html)

Signed-off-by: Jonas Brüstel <jonas.bruestel@piobyte.de>
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

Thank you very much for this improvement. I have left a comment. And I will test it in the next couple of days.

}

if (htmlFormatting) {
parts.add(new StringPart(MESSAGE_KEY_HTML, MESSAGE_VALUE_HTML, UTF_8_ENCODING));
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use StandardCharsets.UTF_8.name(). Would you be so kind to replace it everywhere in this file?

Copy link
Author

Choose a reason for hiding this comment

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

changed it

@cweitkamp
Copy link
Contributor

I maybe see an issue raising. Currently we check for the length of the message plus the title and suppress to send something when the length exceeds 512 characters.

This will happen very quickly when one will start using HTML in his messages. IIRC the used thresholds are not correct anymore. According to https://pushover.net/api#limits we can increase them to 1024 chars for the message and 250 chars for the title.

Signed-off-by: Jonas Brüstel <jonas.bruestel@piobyte.de>
@bruestel
Copy link
Author

Changed title and message length constraints.
2019-04-26 13 41 30

@bruestel
Copy link
Author

CI crashed. How can I trigger a new build?

@cweitkamp
Copy link
Contributor

Thanks for the changes.

How can I trigger a new build?

You can close the PR and reopen it 😉.

private static final String JSON_API_URL = "https://api.pushover.net/1/messages.json";
private static final String JSON_CANCEL_API_URL = "https://api.pushover.net/1/receipts/{receipt}/cancel.json";
private static final String UTF_8_ENCODING = "UTF-8";
private static final String UTF_8_ENCODING = StandardCharsets.UTF_8.name();
Copy link
Contributor

Choose a reason for hiding this comment

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

That is okay but not the idea behind my comment. I maybe could have explained it a little bit more in detail.

I think we do not need this constant here. Rather replace it everywhere in this file. One benefit will be that it will be possible to replace it completely in the IOUtils.toString() methods by using the Charset itself to omit UnsupportedCharsetException (or UnsupportedEncodingException, see commons.io API).

response = IOUtils.toString(httpPost.getResponseBodyAsStream(), UTF_8_ENCODING);

response = IOUtils.toString(httpPost.getResponseBodyAsStream(), UTF_8_ENCODING);

Copy link
Author

Choose a reason for hiding this comment

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

Now I understand what you meant.
Unfortunately, this method isn't available in the current commons.io API version.
The method toString(InputStream, String) in the type IOUtils is not applicable for the arguments (InputStream, Charset)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. We are using 2.2 and the expected method is available from 2.3. What a pity.

Signed-off-by: Jonas Brüstel <jonas.bruestel@piobyte.de>
Signed-off-by: Jonas Brüstel <jonas.bruestel@piobyte.de>
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@kaikreuzer kaikreuzer merged commit 57dc704 into openhab:master Jun 16, 2019
@schrej-zz
Copy link

Hi,
did this went into the 2.5M1? I fail to use ".withHtmlFormatting" and I get an error message, that 'withHtmlFormatting' is not a member of 'org.openhab.action.pushover.internal.Pushover'.

@bruestel
Copy link
Author

bruestel commented Sep 5, 2019

Hi,
did this went into the 2.5M1? I fail to use ".withHtmlFormatting" and I get an error message, that 'withHtmlFormatting' is not a member of 'org.openhab.action.pushover.internal.Pushover'.

I really don't know. I've added the addon as jar file in my installation (2.4). I can send you the jar or you can compile it yourself.

@wborn wborn added this to the 1.14.0 milestone Dec 7, 2019
@kaikreuzer kaikreuzer changed the title [pushover] Add HTML and Monospace formatting option [pushover] Added HTML and Monospace formatting option Dec 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants