I will try to find some time to test the new binding but can/will only start using it in production once Gas has been implemented for my v4 meter. If you confirm this is on your roadmap soon, I will gladly be of help.
Thanks for your help. On the Github issue thread i have uploaded an updated version in which all meters should be supported.
You can test it. If you have a complete P1 telegram from your setup i can also do the first tests for you.
There is no README (yet). It should be, before you can submit a PR in the upstream repo. Mainly to have it included in the documentation website. I will gladly help you getting this written and/or improved.
I try to prevent using Paper UI as much as possible and use manual configuration whenever possible.
I will try to use Paper UI to get things going, however. It shouldn’t be too different from the current one I guess.
Wow, this is great news! I’m currently testing the implementation and it seems to work fine. Installation was a breeze and my (dutch) P1 electricity meter was found instantly. Thank you.
Unfortunately, this will make my work obsolete. I started writing a 2.0 port about a month ago, but only noticed the new binding two days ago. Well, perhaps I can merge some of my changes into your codebase. I noticed you got the ESH thing definitions all written out in XML. That must’ve been quite a lot of work!
Well, a small thing that is an easy fix was something I noticed while profiling my openhab 2.0 installation. I noticed that the P1TelegramParser has a lot of expensive string concatenation leading to some load on the GC and slightly lowered performance. I’m trying to create a PR towards your fork, but it doesn’t seem to work somehow. Your repo doesn’t show up in the list of available PR targets. The commit is here: https://github.com/gedejong/openhab2-addons
Another change I made was to make the serial port reactive instead of polling. A first step towards that is in the ‘test-async’ branch in my repo. This still needs some work, but it would be nice to not have to do a ‘Thread.sleep()’, but use scheduled time-outs and such.
The last change I made was an auto-discovery for the serial port. In essence it would scan available serial ports, try to connect to each of them at different speeds and see if anything ‘responds’. Then again, to see how easy it is to install the binding as it is now, I don’t think it adds much value. Besides, it could actually mess with other serial devices.
Perhaps you could have a look at the ‘test-async’ branch and see what you think?
I incorporated the String concatenation commit. Thank you for this.
I like the asynchronous approach. I think it will solve some corner cases where the configuration or the binding is reloaded and the recovery period resulted in error messages in the log which don’t have a specific meaning since they will recover themselves.
I do however have some questions about the WIP version:
You add listeners to data available, CD and FE. As far as i understand all the DSMR-specifications only TxD and RxD are connected so other event triggers that come from a Serial pin will never be received (like CD, DTR, Ring etc.). The listener for framing error does nothing in your implementation. Adding listeners on FE, Overrun and parity errors could help identifying errors in an early stage and abort parsing till a new frame arrives. Besides that this is a nice pure implementation i don’t see any functional advantages.
I don’t see any reason to listen for the breakinterruptevent’.
In your implementation you have chosen to support multiple listeners. I don’t see the advantage having this support. Do you have a particul reason for this choice?
I will start implementing an async version of the binding.
I agree to not incorporate the last suggestion for serial port detection. Meters < DSMR v4.2 only communicate once every 10 seconds. For reliable you have to wait at least > 10 seconds before the first frame is received. Port detection will claim the port for a minimum of 20 seconds (in the current implementation the detection period is 30 seconds resulting in a claim of 1 minute). This will result in the situation that if the wrong port is claimed it will be unavailable for other bindings. It is possible storing the correct parameters so the detection has only to be done during installation.
So for now i will not add this feature, but i find the idea very interesting.
Looking forward to your input regarding the async implementation
Hey, that’s really fast. I’ll do a bit more profiling with the new binding to see if I can find anything else.
The async commit I made is totally WIP, so the listeners to CD and FE were just to see if I got any signal on them (nope, like you said). But it would really help indeed in detecting errors before they occur during parsing (edit). BTW, I do see the kernel detecting a disconnect (via dmesg), so I was kind of wondering if that could be handled async as well.
The multiple listeners (subscribe and all) is there because I don’t really like ballooning constructors and it seems a bit strange to just add one setter/getter. I guess I get a bit OCD sometimes Also, since it is WIP, I kind of imagined all the error detection to happen async, such that a discovery service and the DSMR meters themselves could be listeners to the state of the port. This would help create that architecture later. Or said in another way: I see the one-listener-per-class-design as a special, more specific case of the multiple listeners design.
Thanks, I hope this helps. If you have intermediate commits, or maybe something I could help with, that’d be fine.
Also, since it is WIP, I kind of imagined all the error detection to happen async, such that a discovery service and the DSMR meters themselves could be listeners to the state of the port. This would help create that architecture later. Or said in another way: I see the one-listener-per-class-design as a special, more specific case of the multiple listeners design.