[smaenergymeter] Fix handling of broadcast frames

[splatch]

SMA Energy Meter improvements

There are two outstanding issues which are closely related to how UDP traffic is handled by SMA energy meter binding.
This PR addresses both by separating several concerns into their own places, separating network communication from data parsing and port management. Starting point for fix were odd values reported by meter binding in #11497. Additionally it introduces support for handling of multiple meters requested in #3429 which is impossible prior provided fixes.

This work wouldn't be possible without amazing analysis made by Thomas: https://community.openhab.org/t/sma-energy-meter-binding-yields-unplausible-values/128180/4.

[openhab-bot]

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

https://community.openhab.org/t/sma-energy-meter-binding-yields-unplausible-values/128180/6

[monnimeter]

👍 Thank you much!

[splatch]

I've updated PR as there are some troubles with amount of changes I introduced. We're clearing issues out in forum topic mentioned earlier.

[splatch]

@lsiepel None of review checklist mention empty method bodies, sets over lists. Use of primitives over complex types is subjective matter related to nullnes, which in above case when elements are mapped by config mapper, is not wrong. I probably could find dozen of such use cases in existing codebase, yet nobody fight with them.

I'll fix the socket leak if I can. I won't touch nit picks. You decide whether you do for users, continuous improvement or continuous waiting.

[lsiepel]

@lsiepel None of review checklist mention empty method bodies, sets over lists. Use of primitives over complex types is subjective matter related to nullnes, which in above case when elements are mapped by config mapper, is not wrong. I probably could find dozen of such use cases in existing codebase, yet nobody fight with them.

I'll fix the socket leak if I can. I won't touch nit picks. You decide whether you do for users, continuous improvement or continuous waiting.

Tried to fix all comments by a uploading a commit to your repository but permissions are lacking. Could you allow me to perform a commit?

[lsiepel]

I now have triage access, but no write access, could you check again, thanks.

[splatch]

@lsiepel done, you should have write access now.

[lsiepel]

If i understood it right, @splatch has some details about a leak and might fix this. I fixed most review comments and have some questions remaining @lolodomo Hopefully we can get this to the finish line. Thanks!

[lolodomo]

Final comments

[lolodomo]

Only remains the question about usage of scheduleAtFixedRate.

https://www.openhab.org/docs/developer/guidelines.html#e-runtime-behavior

[lolodomo]

No answer?

[lsiepel]

No answer?

Seems not, i changed it to scheduleWithFixedDelay as i'm pretty sure that a little variation in the receiving of packets is not going to harm anything. Please continue with review/merging. So we can put all this behind us.

As the configuration is extended with the poperty serialNumber and when this is not set, the SMAEnergyMeterHandler.java will set the Thing to offline during init. A breaking change.

if (serialNumber == null) {
                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_PENDING,
                        "Meter serial number missing");
                return;
            }

I'll add a notice to the distro file.

[lolodomo]

LGTM

[lolodomo]

i changed it to scheduleWithFixedDelay as i'm pretty sure that a little variation in the receiving of packets is not going to harm anything. Please continue with review/merging. So we can put all this behind us.

Thank you @lsiepel for having considered all comments

[cambronf]

The smaenergymeter binding released in 4.2.0M3 shows a huge build up of UDP multicast frames, causing delays of up to half an hour between the real measurment and the reported value in openhab.
This issue has been reported earlier this year:
[https://community.openhab.org/t/sma-energy-meter-binding-yields-unplausible-values/128180/132?u=fcambron]

The broadcast frames fix introduces clearly a new issue, keeping the bindung un-usable.

openhabian@openhabian:~ $ netstat -nau
Active Internet connections (servers and established)
Proto Recv-Q Send-Q Local Address           Foreign Address         State
udp6  165888      0 :::9522                 :::*
udp6  165888      0 :::9522                 :::*
udp6  156672      0 :::9522                 :::*
udp6  172032      0 :::9522                 :::*
udp6  165888      0 :::9522                 :::*
udp6  159744      0 :::9522                 :::*
udp6  153600      0 :::9522                 :::*
udp6  181248      0 :::9522                 :::*

[lolodomo]

This will be clearly better to create a new issue. If not, your message could be simply not seen or forgotten in a short time.