Added legrand-ecocompteur

[raintonr]

Initial release of this adapter for test. Although it seems to work perfectly in my installation :)

[GermanBluefox]

Automated adapter checker

ioBroker.legrand-ecocompteur

• ❗ [E301] Tests on Travis-ci.com are broken. Please fix.
• 👀 [W400] Cannot find "legrand-ecocompteur" in latest repository

Add comment "RE-CHECK!" to start check anew

[raintonr]

I don't know what's going on here, but Travis builds are certainly passing:

Everything passes on adapter-check too (just waiting for this merge):

[Apollon77]

Seems to be a false positive ... sometimes that happens we don't know why ... so ok!

[GermanBluefox]

LGTM

[Apollon77]

Hi,

some review comments from me, all no real blockers

• please check if there are roles matching for your states ... W, kWH and such :-) https://github.com/ioBroker/ioBroker.docs/blob/master/docs/en/dev/stateroles.md
• If you do not need to react on state/object changes please also not implement the methods (unneeded logic)
• using Intervals can be dangerous when there are network problems - ok not that common for local network devices but still :-) more best practice is to use timeouts where next timeout is set after one call is done

Else also looking fine for me; the roles would be cool to get checked

[Apollon77]

Please give a short info if you want to check these or if I should merge into repo

[raintonr]

some review comments from me, all no real blockers

If you want to critique the code for this adapter then you should really open an issue over in that repo:

https://github.com/raintonr/ioBroker.legrand-ecocompteur/issues

That said...

• please check if there are roles matching for your states ... W, kWH and such :-) https://github.com/ioBroker/ioBroker.docs/blob/master/docs/en/dev/stateroles.md

Yes, I looked at that list and there is nothing suitable. The adapter would need:

• Instantaneous power measurement in Watts. Something like value.power or value.power.consumption.instantaneous.
• Cumulative energy usage in kWh. Something like value.energy or value.energy.consumption.cumulative.

I notice there is value.power.consumption in kWh but this is incorrect nomenclature as kWh is a measure of energy and not power. If someone wants to correct that and create something more suitable happy to use those roles.

• If you do not need to react on state/object changes please also not implement the methods (unneeded logic)

Good to know. You will notice I simply took the adapter template and did not touch anything I didn't need to. I left these methods because I didn't know if removing them would break the ioBroker world. If they can be safely removed I will do so.

May I suggest the template should be updated to mention in the comments where code can be safely removed.

• using Intervals can be dangerous when there are network problems - ok not that common for local network devices but still :-) more best practice is to use timeouts where next timeout is set after one call is done

Yes, I thought about that. You will notice the minimum polling period is 1s whereas the HTTP request has a 750ms timeout. No problem as the request will always complete or error before the next is due.

Of course I tested this by pulling the cable out of my router and also pointing the IP address at an incorrect device that would throw errors. Works perfectly.

[GermanBluefox]

It is my fault with power.consumption. Just lost in translation. But it is a good opportunity to correct it.

What do you suggest? When I translate with google, I get "power consumption"...

I could find 'elecrical power consumption' and 'electricity consumption' as official translations too, but I could find 'electric energy consumption' in wiki

[raintonr]

What do you suggest? When I translate with google, I get "power consumption"...

Energy is the ability to do work. Measured in an amount of power spread over a period of time.

Power is the rate of doing work.

This is a good explanation: http://www.differencebetween.net/science/difference-between-energy-and-power/

Perhaps good roles would be:

• value.power
• value.power.consumption
• value.power.production

All in W, kW, etc.

It's good to separate consumption and production because someone might have a Solar PV or other generation system for which they would require that latter one.

The 'instantaneous' part I mentioned before is probably redundant because thinking about it, by it's definition power is a constantly varying value so any reading only applies to that very instant in time it was taken. For example, with the device this adapter is polling, the best you can get is a reading every second so that is a 1 second sample.

And therefore for energy one would need:

• value.energy
• value.energy.consumption
• value.energy.production

All in Wh, kWh, etc.

Maybe you want to include 'electrical' in there? Eg. value.power.electrical.consumption or something? I mention this because we have a heat pump for heating/hot water. There are sensors for water flow and leaving/return temperatures. From the flow and temperature difference it is possible to calculate the thermal power it is producing. Indeed, we measure those values, calculate the thermal power from them and divide by the electrical input power the unit is consuming which gives a COP value.

That said, further grouping these power/energy roles like that is probably overkill and unnecessary ;)

[Apollon77]

Aaahhhh ... did not saw Bluefox answer :-) ... ignore what was here before

[Apollon77]

If you want to critique the code for this adapter then you should really open an issue over in that repo

Basically you rare right, we just did it inthe past so that for the "bring adapter in into latest the first time" we do this initial review here also to dicument that status in one place.

[Apollon77]

PS: Also reading https://github.com/ioBroker/ioBroker.repositories#development-and-coding-best-practices is always a good starting point :-) Should be as comment in new adaoter screated by adater creator or newest templates

[raintonr]

So from all this... is there anything here that someone insists I must change in the adapter before merging this pull request?

[Apollon77]

No I merge it ...

[raintonr]

That's good, thanks.

With regards to the state roles... are those defined anywhere? I mean, I see they are documented in the file you linked... but is there anywhere in the code that actually validates a role to one of these accepted options? Or is it just a string and it will literally store anything you give it?

Just if it's the latter I can simply update my code and it will work.

[Apollon77]

No roles are not validated right now.

[Apollon77]

Ps: but we think doing it ;-)