[dsmr] Add support for Austrian smart meter #11193
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Thomas <thomas@knaller.info>
|
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/dsmr-binding-for-austrian-smart-meters/90649/11 |
Skinah
changed the title
Hilbrand
added a commit
to Hilbrand/openhab-addons
that referenced
this pull request
Oct 1, 2021
Improved the work done in pr openhab#11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
lolodomo
requested changes
Oct 24, 2021
| <description>Production Tariff 2.</description> | ||
| <state pattern="%.1f %unit%" readOnly="true"/> | ||
| </channel-type> | ||
| <channel-type id="totalImportedRnergyRegisterRRate2NonKiloType"> |
There was a problem hiding this comment.
TReplace RRnergy by Energy.
This name is very long (same remark for lots of your added channels). Any chance to reduce it, remaining consistent with other channels ? For example, is the ending "NonKiloType" really useful ? Is "register" useful ?
Comment on lines
+390
to
+391
| <label>Total Imported Energy Rate2 (R+)</label> | ||
| <description>The Total imported energy register Rate2 (R+).</description> |
There was a problem hiding this comment.
Considering the channel name, I guess that you should use "exported" in label and description.
| <channel-type id="deliveryTariff2NonKiloType"> | ||
| <item-type>Number:Energy</item-type> | ||
| <label>Delivery Tariff 2</label> | ||
| <description>Production Tariff 2.</description> |
There was a problem hiding this comment.
The description is probably wrong as exactly the same as for another channel.
| <channel-type id="emeterDeliveryTariff1NonKiloType"> | ||
| <item-type>Number:Energy</item-type> | ||
| <label>Delivery Tariff 1</label> | ||
| <description>Production Tariff 1.</description> |
There was a problem hiding this comment.
The description is probably wrong as exactly the same as for another channel.
Comment on lines
+414
to
+415
| <label>Actual Power Delivery</label> | ||
| <description>Actual Power Delivery.</description> |
There was a problem hiding this comment.
Label and description are strictly the same as for the channel actualProductionNonKiloType. Please explain the difference at least in the description of both channels.
Comment on lines
+402
to
+403
| <label>Total Imported Energy Rate1 (R+)</label> | ||
| <description>The Total imported energy register Rate1 (R+).</description> |
There was a problem hiding this comment.
Considering the channel name, I guess that you should use "exported" in label and description.
| <description>This is an electricity meter that complies to the Austria's Smarty V1.0 specification.</description> | ||
|
|
||
| <channels> | ||
| <channel id="emeter_actual_reactive_delivery" typeId="actualReactiveDeliveryType"/> |
There was a problem hiding this comment.
This is a little strange to use _ for channel id while not for channel type.
Is it consistent with existing channels inj the binding ?
|
|
||
| <channels> | ||
| <channel id="emeter_actual_reactive_delivery" typeId="actualReactiveDeliveryType"/> | ||
| <channel id="emeter_production_tariff1_non_kilo" typeId="productionTariff1NonKiloType"/> |
There was a problem hiding this comment.
Comment for all channels: is the "_non_kilo" really useful ?
Comment on lines
+260
to
+261
|
|
||
|
|
There was a problem hiding this comment.
Please check the SAT results if you have something for these added blank lines.
Normally useless blank lines is not recommended.
Comment on lines
+300
to
+301
|
|
||
|
|
There was a problem hiding this comment.
Same here. To be checked.
|
@Hilbrand : can you please additionally review and comment ? I have some comments which may be relative to consistency with existing channels. |
|
@lolodomo as this pr breaks things I'm not sure what what intended with this pr. However, I've already rewritten the support for Austrian meters (see the link above to my commit that GitHub linked when I pushed my changes), which removes all the additional non kilo channels added, as it's not needed. I'm intending to submit this as new pr, and than this pr can be closed. But I've not had time to submit that pr as I still wanted to let my changes run for some time on my own system as the code also contains some additional improvements and I worked on the Spotify improvements. So I'll hope to submit this pr soon and we can close this pr. |
|
as said in my opening statement: don't use this pull request as. thank you @Hilbrand for incorporating these features in Hilbrand@38f6eac |
Hilbrand
added a commit
to Hilbrand/openhab-addons
that referenced
this pull request
Oct 25, 2021
Improved the work done in pr openhab#11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
Hilbrand
added a commit
to Hilbrand/openhab-addons
that referenced
this pull request
Oct 27, 2021
Improved the work done in pr openhab#11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
fwolter
pushed a commit
that referenced
this pull request
Oct 30, 2021
…11458) * Fix fix for channel id detection, Added missing channels to emucs electra - M-bus channels are dynamic and present in the obis id. The binding had most channel types fixed because most of the time these channels are the same. However the device identifier is the same for multiple devices. But the binding only registered only one and while the channel id was derived from this obis data. For detected meters this resulted in the channel id to be the same if there are multiple devices. This change looks at the channel id to assign it to the found device. This is a bit tricky because the general device has no channel and has channels that have different id's. So the binding needs to cover that case. This change also adds some optional channels to the emucs electra meter. Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Add support for Austrian meters Improved the work done in pr #11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Added Null handling annotations.
dschoepel
pushed a commit
to dschoepel/openhab-addons
that referenced
this pull request
Nov 9, 2021
…penhab#11458) * Fix fix for channel id detection, Added missing channels to emucs electra - M-bus channels are dynamic and present in the obis id. The binding had most channel types fixed because most of the time these channels are the same. However the device identifier is the same for multiple devices. But the binding only registered only one and while the channel id was derived from this obis data. For detected meters this resulted in the channel id to be the same if there are multiple devices. This change looks at the channel id to assign it to the found device. This is a bit tricky because the general device has no channel and has channels that have different id's. So the binding needs to cover that case. This change also adds some optional channels to the emucs electra meter. Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Add support for Austrian meters Improved the work done in pr openhab#11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Added Null handling annotations. Signed-off-by: Dave J Schoepel <dave@theschoepels.com>
jpg0
pushed a commit
to jpg0/openhab-addons
that referenced
this pull request
Nov 10, 2021
…penhab#11458) * Fix fix for channel id detection, Added missing channels to emucs electra - M-bus channels are dynamic and present in the obis id. The binding had most channel types fixed because most of the time these channels are the same. However the device identifier is the same for multiple devices. But the binding only registered only one and while the channel id was derived from this obis data. For detected meters this resulted in the channel id to be the same if there are multiple devices. This change looks at the channel id to assign it to the found device. This is a bit tricky because the general device has no channel and has channels that have different id's. So the binding needs to cover that case. This change also adds some optional channels to the emucs electra meter. Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Add support for Austrian meters Improved the work done in pr openhab#11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Added Null handling annotations.
NickWaterton
pushed a commit
to NickWaterton/openhab-addons
that referenced
this pull request
Dec 30, 2021
…penhab#11458) * Fix fix for channel id detection, Added missing channels to emucs electra - M-bus channels are dynamic and present in the obis id. The binding had most channel types fixed because most of the time these channels are the same. However the device identifier is the same for multiple devices. But the binding only registered only one and while the channel id was derived from this obis data. For detected meters this resulted in the channel id to be the same if there are multiple devices. This change looks at the channel id to assign it to the found device. This is a bit tricky because the general device has no channel and has channels that have different id's. So the binding needs to cover that case. This change also adds some optional channels to the emucs electra meter. Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Add support for Austrian meters Improved the work done in pr openhab#11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Added Null handling annotations. Signed-off-by: Nick Waterton <n.waterton@outlook.com>
nemerdaud
pushed a commit
to nemerdaud/openhab-addons
that referenced
this pull request
Jan 28, 2022
…penhab#11458) * Fix fix for channel id detection, Added missing channels to emucs electra - M-bus channels are dynamic and present in the obis id. The binding had most channel types fixed because most of the time these channels are the same. However the device identifier is the same for multiple devices. But the binding only registered only one and while the channel id was derived from this obis data. For detected meters this resulted in the channel id to be the same if there are multiple devices. This change looks at the channel id to assign it to the found device. This is a bit tricky because the general device has no channel and has channels that have different id's. So the binding needs to cover that case. This change also adds some optional channels to the emucs electra meter. Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Add support for Austrian meters Improved the work done in pr openhab#11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Added Null handling annotations.
marcfischerboschio
pushed a commit
to bosch-io/openhab-addons
that referenced
this pull request
May 5, 2022
…penhab#11458) * Fix fix for channel id detection, Added missing channels to emucs electra - M-bus channels are dynamic and present in the obis id. The binding had most channel types fixed because most of the time these channels are the same. However the device identifier is the same for multiple devices. But the binding only registered only one and while the channel id was derived from this obis data. For detected meters this resulted in the channel id to be the same if there are multiple devices. This change looks at the channel id to assign it to the found device. This is a bit tricky because the general device has no channel and has channels that have different id's. So the binding needs to cover that case. This change also adds some optional channels to the emucs electra meter. Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Add support for Austrian meters Improved the work done in pr openhab#11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Added Null handling annotations.
andan67
pushed a commit
to andan67/openhab-addons
that referenced
this pull request
Nov 6, 2022
…penhab#11458) * Fix fix for channel id detection, Added missing channels to emucs electra - M-bus channels are dynamic and present in the obis id. The binding had most channel types fixed because most of the time these channels are the same. However the device identifier is the same for multiple devices. But the binding only registered only one and while the channel id was derived from this obis data. For detected meters this resulted in the channel id to be the same if there are multiple devices. This change looks at the channel id to assign it to the found device. This is a bit tricky because the general device has no channel and has channels that have different id's. So the binding needs to cover that case. This change also adds some optional channels to the emucs electra meter. Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Add support for Austrian meters Improved the work done in pr openhab#11193 Also-by: Thomas <thomas@knaller.info> Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl> * [dsmr] Added Null handling annotations.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support for Austrian Smart Meter. Actually breaks everything, please do not approve pull request (failing tests make sure of that).
the issue is, that the austrian meters use a different quantitytype for the same OBISIdentifier
for example:
1-0:1.8.1(000230515*Wh)
vs:
1-0:1.8.1(123456.789*kWh)
which makes parsing/detecting impossible with the current system in the binding.