[nuvo] Nuvo Whole House Audio Binding - initial contribution

[mlobstein]

[nuvo] Initial contribution

This is the initial implementation of a Binding to control the Nuvo Grand Concerto or Essentia G whole house multi-zone amplifier.

[mlobstein]

Test build available here:
https://github.com/mlobstein/mlobstein-beta-test/raw/master/org.openhab.binding.nuvo-2.5.8-SNAPSHOT.jar

If you receive an error about openhab-transport-serial, issue the following command in the openhab console:

feature:install openhab-transport-serial

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[mlobstein]

@fwolter
Hold off on reviewing until #7371 is sorted out. This binding's Connector classes are of the same derivative, so I will need to make the requested changes here also.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[fwolter]

Thanks again! There are some compiler warnings left, concerning null pointers.

[fwolter]

Is it worth to translate odd values to even values, instead of ignoring them?

[mlobstein]

The device is weird in that it only accepts even values (from -18 to 18). So this was done to along with step=2 in the channel configuration to have the binding values match the device. I noted it in the readme also.

[fwolter]

I had something like this in mind:

if(value % 2 == 1) value++;

[mlobstein]

good idea, it will automatically become a compatible value.

[fwolter]

Did you try this with different system timezones and configuring the timezone in PaperUI? I saw an example retrieving a zoned datetime service or so via @Reference in an openHAB binding, but I can't find it again.

[mlobstein]

I did not. It was just intended to use whatever the system time is regardless of time zone. I think this will be fine since most users set the host system to their local time.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[Hilbrand]

The openHAB serial port handler supports rfc2217? Is there a reason not to use it and instead of having implemented a separate ip connector and configuration options?

[mlobstein]

I am not sure as this was pulled in from the rotel binding. I seems like rfc2217 (Telnet Com Port) may be supported to some degree. But there were probably issues, so a distinct connector was defined instead and this check was put in place to prevent an unusable configuration. @lolodomo any ideas?

[lolodomo]

RFC2217 through the openhab serial layer never worked in bindings I worked on.
I am not sure of the reason but my hypothesis was it doesn't work if you need to read and write in parallel.
But you are welcome to try with any new binding. Maybe I was just not enough clever to make it work.

[lolodomo]

This is probably a problem in case you have a reader thread that always read data.

[mlobstein]

That makes sense. But I think I will just stick with what works.

[lolodomo]

@hillbrand: Honestly I don't remember. I even don't remember if I tested that with this binding in particular. But as I am using serial over IP, I could try again one day.

[mlobstein]

@lolodomo Maybe it was related to the endless sleep 'fix' (that could be and was removed during the review process)? Did you ever test after that change?

@mlobstein Did you test if it worked at all by using the rfc2217 version?
I did not test it with rfc2217

[Hilbrand]

If it's not tested or not known if the rfc2217 configuration way doesn't work I would be in favor of removing the specific ip implementation here and allowing the rfcc2217 configuratiin option for serial. Its always possible to add features, but removing them is hard. This also reduces the amount of copy-pasting of specific code.

[mlobstein]

A couple questions...

1. I tried rfc2217 and it seemed to work connecting to a ser2net endpoint after a few changes to the serial connector. I had to conditionally bypass SerialPort.enableReceiveThreshold(), SerialPort.enableReceiveTimeout() & SerialPort.setFlowControlMode() as the calling those with rfc2217 throws an exception. I also had to catch an IllegalStateException that was being thrown if the remote connection was refused or otherwise failed to connect. That being said, are there any bindings that currently support rfc2217 explicitly (as a reference implementation)? I did a quick scan of the 2.5.x bindings and the only references to rfc2217 were in 6 other bindings that blocked its usage during initialization.

2. If rfc2217 is to be used, does the user need to make the choice to specify the rfc2217 url string in place of the serial port or can this be handled behind the scenes? There seems to be some controversy with setting limitToOptions=false for the serial port in the configuration. [Q] How to add a rfc2217://.. port in the UI (PaperUI, Habmin) openhab-core#1029 Once this is set to false, the user can type the url string for the rfc2217 connection (rfc2217://:). I feel this is cumbersome and it would be much easier for the user to give them a place to specify IP and port # for the remote serial port and use those to construct the rfc2217 url in the absence of a physical serial port being selected from the drop down.
   @kaikreuzer

[Hilbrand]

Thank you for extensive testing this. At least we know it works (besides the exception throwing. In know the dsmr binding does also works once the exceptions would be wrapped.). The openhab-core issue 1029 you mentioned was about the possibility to be able to add a network connection. It doesn't even address the usability of it, which is too bad. I also looked at the other bindings. It looks to me this has grown historically. In the past the gnu serial library was used directly, so no rfc2217 support was availble. So that earlier binding implemented their own version of it. Later versions seem to be made by the same person and he copied that implementation. The other bindings seem to add a bridge for ip configuration. I also noted we already approved your oppo binding that does the same.

So I'll guess it should be ok to leave it as is here. What I'm planning, is to see if I can get those quircks fixed in core so it behaves the same as other direct serial connections and than see if this makes it possible to remove the custom implementations for ip connections. For example here you could construct the rfc2217 connection fromt host-port input and then use the rfc2217 of core. The configurations can than still remain (with addition that rfc2217 configuration would be allowed on serialPort configuration). But that would not be available before openHAB 3.0.

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests have failed

Hey @mlobstein,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

[Hilbrand]

LGTM

[TravisBuddy]

Travis tests were successful

Hey @mlobstein,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.