volkszaehler / mbmd

ModBus Measurement Daemon - simple reading of data from ModBus meters and grid inverters
BSD 3-Clause "New" or "Revised" License
229 stars 81 forks source link

Orno1p: wrong opcodes? #283

Open andig opened 1 year ago

andig commented 1 year ago

@ochykysh could you kindly double-check https://github.com/volkszaehler/mbmd/blob/master/meters/rs485/orno1p.go? It seems strange to have 3p values in the opcodes map for a 1p meter? Also seems as if not all opcodes are referenced by the producer?

Stumbled upon the by user report of https://github.com/evcc-io/evcc/issues/4576

Originally posted by @andig in https://github.com/volkszaehler/mbmd/issues/129#issuecomment-1257151243

ochykysh commented 1 year ago

(copy of comment in the original issue)

@andig The register codes are from the original PDFs

https://b2b.orno.pl/download-resource/26063/ https://b2b.orno.pl/download-resource/26064/

And yes, they do have 3p registers on a 1p meter...no idea why. I decided not to poll for registers that will be empty, so that's why data is not being collected from phase 2 and phase 3.

We could remove the references if you think it would be better.

andig commented 1 year ago

Thank you, seems the confusion is due to the vendor. Main problem is that Power won't work on the 1p meter. Seems we should make Power an alias of PowerL1 here?

ochykysh commented 1 year ago

If we do that, probably makes sense to do it for

VoltageL1 -> Voltage
CurrentL1 -> Current
PowerL1 -> Power
ReactivePowerL1 -> ReactivePower
ApparentPowerL1 -> ApparentPower
CosphiL1 -> Cosphi

Do we have some alias implementation already? Don't want to poll all of those registers two times unless I have to.

andig commented 1 year ago

Do we have some alias implementation already?

We don't and it would probably not help. Problem seems to be that for the "official" registers the L1 measurements use different registers from the total measurements.

As the implementation is targeted at L1 meters my current approach would be:

Downside: we will no longer show L1 measurements in the UI. OR we duplicate them, but that would generate additional reads.

Seems we need to think about a proper solution a little more.

ochykysh commented 1 year ago

Just a note: if we just rename them, we will break existing installations.
Too bad we can't return the set of values as a whole, that way we could easily add the alias implementation.

RichieB2B commented 1 year ago

Too bad we can't return the set of values as a whole, that way we could easily add the alias implementation.

We and we should, as proposed in #255