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

Conversation

@TheNetStriker
Copy link
Contributor

  • Adds a new setting to select charset for string encoding.
  • Problem parsing more than one setting is solved.
  • The BASE64 setting can now also be written as BASE64() and then it
    will be parsed correctly at any position inside the string.
  • Readme updated regarding the new charset setting and the new BASE64
    settings format.

Forum link: https://community.openhab.org/t/strange-problem-with-serial-binding/43230

Signed-off-by: David Masshardt david@masshardt.ch (github:
TheNetStriker)

Copy link
Contributor

@9037568 9037568 left a comment

Choose a reason for hiding this comment

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

I only see a few minor tweaks as needed.
Looks like a great fix.

}

public SerialDevice(String port) {
public SerialDevice(String port, String charsetName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a public API, we should not destroy the original constructor. Please just add a new one.

}

public SerialDevice(String port, int baud) {
public SerialDevice(String port, int baud, String charsetName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment about constructors above.

private void setCharset(String charsetName) {
try {
if (charsetName == null) {
this.charset = Charset.defaultCharset();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should eliminate the extraneous use of this. throughout.


logger.debug("Serial port '{}' charset '{}' set.", this.port, charsetName);
} catch (IllegalCharsetNameException e) {
logger.error("Serial port '{}' charset '{}' not found.", this.port, charsetName);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a WARN, not an ERROR.
Also, I think there should probably be logic here to set the charset to the default when this exception is caught.

* `BASE64()` enables the Base64 mode. With this mode all data received on the serial port is saved in Base64 format. All data that is sent to the serial port also has to be Base64 encoded. (This was implemented because some serial devices are using bytes that are not supported by the REST interface).
* `ON(<On string>),OFF(<Off string>)` used in conjunction with a Switch, this mapping will send specific commands to serial port and also match a serial command to specific ON/OFF state. This makes it unnecessary to use a rule to send a command to serial.
* `UP(<Up string>),DOWN(<Down string>),STOP(<Stop string>)` used in conjunction with a Rollershutter, this mapping will send specific commands to serial port. Use REGEX to parse Rollershutter postion (0-100%) coming as feedback over serial link.
* `CHARSET(<charset>)` set's the charset to be useed for converting to a String and back to bytes when writing. (e.g. UTF-8, ISO-8859-1, etc.)
Copy link
Contributor

@9037568 9037568 Aug 10, 2018

Choose a reason for hiding this comment

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

One more:
"sets the charset to be used"

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also change the first sentence in the README to eliminate the reference to ASCII ?

- Adds a new setting to select charset for string encoding.
- Problem parsing more than one setting is solved.
- The BASE64 setting can now also be written as BASE64() and then it
will be parsed correctly at any position inside the string.
- Readme updated regarding the new charset setting and the new BASE64
settings format.

Signed-off-by: David Masshardt <david@masshardt.ch>
@TheNetStriker
Copy link
Contributor Author

@9037568 I've just uploaded the requested changes. Please check again if everything is ok now.

Copy link
Contributor

@9037568 9037568 left a comment

Choose a reason for hiding this comment

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

LGTM.

@9037568 9037568 added this to the 1.13.0 milestone Aug 14, 2018
@9037568
Copy link
Contributor

9037568 commented Aug 18, 2018

Thanks, @TheNetStriker !

@9037568 9037568 merged commit 015c09a into openhab:master Aug 18, 2018
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/serial-binding-sends-close-bracket-after-update-to-2-3/48801/5

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.

3 participants