Skip to content
This repository has been archived by the owner on May 17, 2021. It is now read-only.

[TCP Binding] Enhancements for README.md #5710

Merged
merged 4 commits into from Dec 21, 2018
Merged

Conversation

arberg
Copy link
Contributor

@arberg arberg commented Nov 17, 2018

Signed-off-by: Alex R. Berg, developlantern@gmail.com, github: arberg

Better example, info that its broken with links, added unclear stuff - strange to have in wiki, but better than leaving user in wilderness I believe. Lets just be honest, if somebody knows what it means and feels like specified that would be great.

Better example, info that its broken with links, added unclear stuff - strange to have in wiki, but better than leaving user in wilderness I believe. Lets just be honest, if somebody knows what it means and feels like specified that would be great.
fixed links
@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/tcp-binding-multiple-items-from-single-ip-port/36490/6

@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/wiki-update-failed-automatic-jenkins-tests/56968/1

@9037568 9037568 changed the title Update README.md [TCP Binding] Enhancements for README.md Nov 18, 2018
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.

Thanks for the documentation enhancements. Please see my inline comments.

@@ -2,9 +2,11 @@

The TCP and UDP bindings provide basic support for simple ASCII-based protocols. They send and receive data as ASCII strings. Data sent out is by default padded with a CR/LF. This should be sufficient for many home automation devices that take simple ASCII-based control commands, or that send back text-based status messages.

The TCP part of the binding has a built-in mechanism to keep connections to remote hosts alive, and will reset connections at regular intervals to overcome the limitation of "stalled" connections or remote hosts.
The TCP part of the binding has a built-in mechanism to keep connections to remote hosts alive, and will reset connections at regular intervals to overcome the limitation of "stalled" connections or remote hosts. (see broken comment at the bottom)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nope. Remove this.


The TCP & UDP Bindings act as a network client or as a network server.
The TCP & UDP Bindings act as a network client (outgoing direction ">[]" config) or as a network server (incoming direction "<[]").
Copy link
Contributor

Choose a reason for hiding this comment

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

Too in the weeds. Revert to original.

The TCP & UDP Bindings act as a network client or as a network server.
The TCP & UDP Bindings act as a network client (outgoing direction ">[]" config) or as a network server (incoming direction "<[]").

The TCP & UDP Binding has a single config file for tcp.cfg and another for udp.cfg, and cannot have different parameters for different hosts.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what this is trying to say. If you can clarify it, please move it below the section on Binding Configuration.

@@ -29,8 +31,8 @@ The indicated default values apply to both bindings unless otherwise noted.
| timeout | 3000 | No | Timeout, in milliseconds, to wait for a reply when initiating a blocking write/read operation |
| updatewithresponse | false | No |Update the status of items using the response received from the remote end (if the remote end sends replies to commands) |
| itemsharedconnections | false | No | Set to `true` to share connections within the item binding configurations |
| bindingsharedconnections | false | No | Set to `true` to share connections between item binding configurations |
| directionssharedconnections | true | No | Set to `false` to not share connections between inbound and outbound connections |
| bindingsharedconnections | false | No | Set to `true` to share connections between item binding configurations: If true will imply itemsharedconnections=true (and give warning if it was false) |
Copy link
Contributor

Choose a reason for hiding this comment

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

This addendum and the one below it should be clarified and moved to prose below the table.

@@ -42,14 +44,15 @@ The format of the binding configuration is simple and looks like this:

```
tcp="<direction>[<command>:<ip address>:<port>:<transformationrule>], <direction>[<command>:<ip address>:<port>:<transformationrule>], ..."
tcp="<direction>[<ip address>:<port>:<transformationrule>]"
Copy link
Contributor

Choose a reason for hiding this comment

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

To make this more clear, let's break this into two examples, one for IN and one for OUT, with appropriate labels.

@@ -68,4 +71,25 @@ Here are some examples of valid binding configuration strings:
```
tcp=">[ON:192.168.0.1:3000:'MAP(my.device.map)'], >[OFF:192.168.0.1:3000:'MAP(my.device.map)']" // for a Switch Item where values are converted using the my.device.map
tcp="<[192.168.0.2:3000:'REGEX((.*))']" // for a String Item that captures some state of a remote device that connects to openHAB
tcp=">[192.168.0.2:3000:]" // same as REGEX above but outgoing, it connects to remote port, and sends any commands received
```
Here's a full item configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

"configuration:"

```
The TCP Binding will open a port to remote host 10.0.0.4:7000. Any commands received on item will be sent to remote with pre and postfix specified by preamble and postamble. Item will get postUpdate on any data received from remote via the tcp connection. The data can be mapped via rules to virtual items as MyVirtualLyngdorfPowerSwitch
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a blank line below ending code fence.

```
The TCP Binding will open a port to remote host 10.0.0.4:7000. Any commands received on item will be sent to remote with pre and postfix specified by preamble and postamble. Item will get postUpdate on any data received from remote via the tcp connection. The data can be mapped via rules to virtual items as MyVirtualLyngdorfPowerSwitch
Copy link
Contributor

Choose a reason for hiding this comment

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

"Any commands received on the item will be sent to the remote..."
"The item will get postUpdate on any data received from the remote..."
"TCP connection"
"virtual items such as MyVirtualLyngdorfPowerSwitch."

```
The TCP Binding will open a port to remote host 10.0.0.4:7000. Any commands received on item will be sent to remote with pre and postfix specified by preamble and postamble. Item will get postUpdate on any data received from remote via the tcp connection. The data can be mapped via rules to virtual items as MyVirtualLyngdorfPowerSwitch

## Currently broken
Copy link
Contributor

Choose a reason for hiding this comment

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

This section should be removed.

It may be replaced by a section titled "Known Issues" if you wish, with a link to issue #2706.


Also for an alternative solution using MTQQ and a small python server delivering data to MTQQ message broker, see JGluch example here: https://community.openhab.org/t/solved-optoma-beamer-via-rs232-over-tcp-ip-connector/38719/10

## Unclear stuff
Copy link
Contributor

Choose a reason for hiding this comment

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

This section should be removed. "Unclear stuff" should be clarified via the community forum and then we can add "clear stuff" here.

@arberg
Copy link
Contributor Author

arberg commented Nov 23, 2018

Hi Chris,

That is a lot of edits you have requested. It is a very slow and heavy process, if I am to understand what you mean, locate it manually and then apply the edit, and hope I got it all. I don't seem to be able to find an 'apply' button for the comments, probably because they are just comments. I also havn't found a nice side by side review, and when I checked out my fork of the main-repo, my changes to the readme wasn't there, so can use intellij/sourcetree to view changes.

I enjoy sharing what I have learned so others don't have to go through the same debugging process reviewing 10s of other peoples questions on community forum discussing the seemingly same isuses. I also like clarifying missing examples etc. But its gotta be quick for me, otherwise I won't do it.

I don't mind you editting my edits, just delete what you don't like, and edit what you do. I can review your review, if needed.

I can also post this on community forum if you wish.

I figured I might go over the other docs where I had trouble getting it to work, such as network binding ping, and mqtt broker, if it was super easy for me.

Regarding the 'broken' stuff: I personally find it more helpful to have more information. It makes sense to label it 'known issues'. In my book, its bad for publicity to claim something works, and then when people poke around there are issues everywhere. I'm new to openHab, and might not even stay because the getting started has been fraught with weird behaviours, like even a simple ping didn't work out-of-the-box. But I do absolutely love the rule language and the power openHAB gives, which I suspect is not quite there for Home Assistent. My message being: Lets make it easy on new users, by giving them the info they need where they look for it, if possible. Going over community forums for known issues that nobody seems to 'own' is a lot of work. Like if TCP binding sucks stability wise, we can show them MQTT and make MQTT doc really good.

Best Alex

@9037568
Copy link
Contributor

9037568 commented Nov 23, 2018

But its gotta be quick for me, otherwise I won't do it.

I get the picture. I'll leave this open for a while just in case you decide to spend a few seconds on it.

@arberg
Copy link
Contributor Author

arberg commented Dec 3, 2018

Hi Chris, I figured out GitHub kept the changes in a branch, so I could pull that and edit locally. That helped a bit with diffing and applying changes. I have added/updated with most of your comments. I may have missed some given the nature of the task. I chose not to follow your guideline regarding breaking the syntax (line 47) into two examples for IN and OUT. I kept it this way to avoid duplicating the explanations. I now see you probably meant replace '' with '<'. I still don't think that help much. Instead I have improved much upon the examples, splitting those into IN and OUT. I elaborate the difference between direction IN/OUT and command/update (data in/out). Its very easy to get confused, I certainly was and was mislead by other confused people on the forum.

If you want find minor edits and its easy to do, please feel free to just make the edits, and send me the whole changed file, and just leave inline comments for bigger things, if any.

I can of cause delete the section on known isuses and alternative solutions, but I'm quite sure that would not help the adopters of OpenHAB.

Best Alex

@9037568 9037568 merged commit 5fd14b5 into openhab:master Dec 21, 2018
@9037568
Copy link
Contributor

9037568 commented Dec 21, 2018

Thanks, @arberg !

@9037568 9037568 added this to the 1.13.0 milestone Dec 21, 2018
@arberg arberg deleted the patch-1 branch January 3, 2019 16:36
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.

None yet

3 participants