[fronius] Added inverter power, battery state of charge and PV solar yield #10757

THKDev · 2021-05-24T13:20:46Z

add missing field of solar yield (PV)
add flow data of inverter power
add flow data of battery state of charge

fwolter · 2021-06-05T12:39:14Z

@trokohl Can you take a look?

trokohl · 2021-06-05T12:55:54Z

looks good

fwolter · 2021-06-05T13:02:43Z

...ding.fronius/src/main/java/org/openhab/binding/fronius/internal/FroniusBindingConstants.java

@@ -47,6 +48,7 @@
    public static final String PowerFlowpGrid = "powerflowchannelpgrid";
    public static final String PowerFlowpLoad = "powerflowchannelpload";
    public static final String PowerFlowpAkku = "powerflowchannelpakku";
+    public static final String PowerFlowpPv = "powerflowchannelppv";


Actually constants should be all upper case. Would be awesome if you could fix all in this class.

Suggested change

public static final String PowerFlowpPv = "powerflowchannelppv";

public static final String POWER_FLOW_P_PV = "powerflowchannelppv";

fwolter · 2021-06-05T13:03:05Z

bundles/org.openhab.binding.fronius/src/main/resources/OH-INF/thing/thing-types.xml

@@ -196,6 +199,30 @@
 		<description>Battery Power ( + charge, - discharge )</description>
 		<state pattern="%.2f W" readOnly="true"></state>
 	</channel-type>
+	<channel-type id="pPv">
+		<item-type>Number</item-type>
+		<label>Solar plant power</label>


Words in labels should be capitalized (except prepositions and so on). See https://www.openhab.org/docs/developer/bindings/thing-xml.html#formatting-labels-and-descriptions

fwolter · 2021-06-05T13:03:27Z

bundles/org.openhab.binding.fronius/src/main/resources/OH-INF/thing/thing-types.xml

+		<item-type>Number</item-type>
+		<label>Solar plant power</label>
+		<description>Current solar plant power</description>
+		<state pattern="%.2f W" readOnly="true"></state>


Suggested change

<state pattern="%.2f W" readOnly="true"></state>

<state pattern="%.2f %unit%" readOnly="true"></state>

I tried the thing with the %unit% but got some strange behavior.

thing-types.xml looks like

<channel-type id="inverter1Soc"> <item-type>Number:Dimensionless</item-type> <label>Inverter 1 State of Charge</label> <description>Inverter 1 State of Charge</description> <state pattern="%.1f %unit%" readOnly="true"></state> </channel-type>

Code looks like (getSoc() returns double)

new QuantityType<>(inverters.get(number).getSoc(), Units.PERCENT);

Debugger

NumberQuantity = "91.5999984741211 %" value = 91.5999984741211 unit = "%" scale = "RELATIVE"

Output switching like this

[INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'FroniusSymoInverter_Inverter1StateofCharge' changed from 950999984741211 % to 95 % [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'FroniusSymoInverter_Inverter1StateofCharge' changed from 95 % to 949000015258789 % [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'FroniusSymoInverter_Inverter1StateofCharge' changed from 9169999694824219 % to 915999984741211 %

What do i miss?

Are you sure your Item configuration is correct? Seems like the Item is updated from two different sources.

Some more investigation shows the following behavior.

i18n.config

:org.apache.felix.configadmin.revision:=L"5" language="de" measurementSystem="SI" service.pid="org.openhab.i18n" timezone="Europe/Berlin"

server locale

LANG=de_DE.UTF-8 LC_CTYPE="de_DE.UTF-8" LC_NUMERIC="de_DE.UTF-8" LC_TIME="de_DE.UTF-8" LC_COLLATE="de_DE.UTF-8" LC_MONETARY="de_DE.UTF-8" LC_MESSAGES="de_DE.UTF-8" LC_PAPER="de_DE.UTF-8" LC_NAME="de_DE.UTF-8" LC_ADDRESS="de_DE.UTF-8" LC_TELEPHONE="de_DE.UTF-8" LC_MEASUREMENT="de_DE.UTF-8" LC_IDENTIFICATION="de_DE.UTF-8" LC_ALL=

Running the openhab server with these settings will result in messed up values like "19563232421875,00" instead of "1956,32".

Running the server with the LC_CTYPE set to en_GB.UTF-8 will result in the correct values "1956.32" but in the wrong number format (point instead of comma).

bash> locale LANG=de_DE.UTF-8 LC_CTYPE="de_DE.UTF-8" LC_NUMERIC="de_DE.UTF-8" LC_TIME="de_DE.UTF-8" LC_COLLATE="de_DE.UTF-8" LC_MONETARY="de_DE.UTF-8" LC_MESSAGES="de_DE.UTF-8" LC_PAPER="de_DE.UTF-8" LC_NAME="de_DE.UTF-8" LC_ADDRESS="de_DE.UTF-8" LC_TELEPHONE="de_DE.UTF-8" LC_MEASUREMENT="de_DE.UTF-8" LC_IDENTIFICATION="de_DE.UTF-8" LC_ALL= bash> LC_CTYPE=en_GB.UTF-8 ./start_debug.sh

So it looks like an issue with the localization of "QuantityType". I think the whole localization should respect the browser languague instead of the server locale.

I can hardly belive this behavior is caused by the locale setting. Maybe it's only a symptom. Did you try this with a fresh installation of OH or on another system?

With 3.1.0-RC1 the localization issue is gone. See latest patch 76d8b02 .

fwolter · 2021-06-30T09:06:29Z

Great that the local issue is gone! Your code doesn't build as it contains a checkstyle error. You could take a look at target/code-analysis/report.html.

Signed-off-by: THKDev <THKDev@users.noreply.github.com>

…s should be capitalized (except prepositions and so on). Use %unit% placeholder. Signed-off-by: THKDev <THKDev@users.noreply.github.com>

…ils) Signed-off-by: THKDev <THKDev@users.noreply.github.com>

fwolter

Your code doesn't compile as it contains a checkstyle error. You could take a look at target/code-analysis/report.html.

fwolter · 2021-07-10T11:59:27Z

...nius/src/main/java/org/openhab/binding/fronius/internal/handler/FroniusBaseThingHandler.java

        } catch (IOException | IllegalStateException e) {
            errorMsg = e.getMessage();
-            logger.debug("Error running fronius request: {}", errorMsg);
+            logger.error("Error running fronius request: {}", errorMsg);


Bindings should only log to error if something severe happened, like the detection of a bug in your code. This could be debug as you catch an IOException. See this link for a description of the log levels: https://www.openhab.org/docs/developer/guidelines.html#f-logging

fwolter · 2021-07-10T11:59:59Z

...onius/src/main/java/org/openhab/binding/fronius/internal/FroniusBaseDeviceConfiguration.java

+    @Nullable
    public Integer deviceId;


Syntactical sugar. Please check all.

Suggested change

@Nullable

public Integer deviceId;

public @Nullable Integer deviceId;

fwolter · 2021-07-10T12:04:23Z

....fronius/src/main/java/org/openhab/binding/fronius/internal/handler/FroniusMeterHandler.java

+    private MeterRealtimeResponseDTO getMeterRealtimeData(final String ip, final Integer deviceId) {
+        Objects.requireNonNull(ip, "IP address must be set in the configuration.");
+        Objects.requireNonNull(deviceId, "Device ID must be set in the configuration.");


This will throw an unchecked exception, which will crash your binding. You could throw a custom exception and catch it in the calling method.

fwolter · 2021-07-10T12:05:55Z

...s/src/main/java/org/openhab/binding/fronius/internal/handler/FroniusSymoInverterHandler.java

-    private PowerFlowRealtimeResponse getPowerFlowRealtime(String ip) {
-        String location = FroniusBindingConstants.POWERFLOW_REALTIME_DATA.replace("%IP%", StringUtils.trimToEmpty(ip));
+    private PowerFlowRealtimeResponse getPowerFlowRealtime(final String ip) {
+        Objects.requireNonNull(ip, "IP address must be set in the configuration.");


See above. Throw custom exception. Please check all.

…StringUtils)" This reverts commit 73065c6 Signed-off-by: THKDev <THKDev@users.noreply.github.com>

Signed-off-by: THKDev <THKDev@users.noreply.github.com>

fwolter · 2021-07-10T16:38:27Z

...s/src/main/java/org/openhab/binding/fronius/internal/handler/FroniusSymoInverterHandler.java


+import org.apache.commons.lang3.StringUtils;


Since we want to get rid of Apache Commons, can you replace this with native Java code? See #7722.

Signed-off-by: THKDev <THKDev@users.noreply.github.com>

fwolter

Thanks! LGTM

…yield (openhab#10757) * [fronius] add battery state of charge and PV yield. Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] Actually constants should be all upper case. Words in labels should be capitalized (except prepositions and so on). Use %unit% placeholder. Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] fixed checkstyle error; removed some warnings (eg. StringUtils) Signed-off-by: THKDev <THKDev@users.noreply.github.com> * Revert "[fronius] fixed checkstyle error; removed some warnings (eg. StringUtils)" This reverts commit 73065c6 Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] fixed checkstyle error Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] removed org.apache.commons.lang3.StringUtils Signed-off-by: THKDev <THKDev@users.noreply.github.com>

…yield (openhab#10757) * [fronius] add battery state of charge and PV yield. Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] Actually constants should be all upper case. Words in labels should be capitalized (except prepositions and so on). Use %unit% placeholder. Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] fixed checkstyle error; removed some warnings (eg. StringUtils) Signed-off-by: THKDev <THKDev@users.noreply.github.com> * Revert "[fronius] fixed checkstyle error; removed some warnings (eg. StringUtils)" This reverts commit 73065c6 Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] fixed checkstyle error Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] removed org.apache.commons.lang3.StringUtils Signed-off-by: THKDev <THKDev@users.noreply.github.com> Signed-off-by: Łukasz Dywicki <luke@code-house.org>

This was added in openhab#10757 but the documentation was not updated. Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

…yield (openhab#10757) * [fronius] add battery state of charge and PV yield. Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] Actually constants should be all upper case. Words in labels should be capitalized (except prepositions and so on). Use %unit% placeholder. Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] fixed checkstyle error; removed some warnings (eg. StringUtils) Signed-off-by: THKDev <THKDev@users.noreply.github.com> * Revert "[fronius] fixed checkstyle error; removed some warnings (eg. StringUtils)" This reverts commit 73065c6 Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] fixed checkstyle error Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] removed org.apache.commons.lang3.StringUtils Signed-off-by: THKDev <THKDev@users.noreply.github.com> Signed-off-by: Łukasz Dywicki <luke@code-house.org>

…yield (openhab#10757) * [fronius] add battery state of charge and PV yield. Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] Actually constants should be all upper case. Words in labels should be capitalized (except prepositions and so on). Use %unit% placeholder. Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] fixed checkstyle error; removed some warnings (eg. StringUtils) Signed-off-by: THKDev <THKDev@users.noreply.github.com> * Revert "[fronius] fixed checkstyle error; removed some warnings (eg. StringUtils)" This reverts commit 73065c6 Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] fixed checkstyle error Signed-off-by: THKDev <THKDev@users.noreply.github.com> * [fronius] removed org.apache.commons.lang3.StringUtils Signed-off-by: THKDev <THKDev@users.noreply.github.com>

THKDev requested a review from trokohl as a code owner May 24, 2021 13:20

kaikreuzer added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jun 5, 2021

trokohl approved these changes Jun 5, 2021

View reviewed changes

fwolter requested changes Jun 5, 2021

View reviewed changes

THKDev added 3 commits July 5, 2021 17:14

[fronius] add battery state of charge and PV yield.

71d6345

Signed-off-by: THKDev <THKDev@users.noreply.github.com>

[fronius] Actually constants should be all upper case. Words in label…

50fa24b

…s should be capitalized (except prepositions and so on). Use %unit% placeholder. Signed-off-by: THKDev <THKDev@users.noreply.github.com>

[fronius] fixed checkstyle error; removed some warnings (eg. StringUt…

73065c6

…ils) Signed-off-by: THKDev <THKDev@users.noreply.github.com>

THKDev force-pushed the feature/fronius_soc branch from 3173781 to 73065c6 Compare July 5, 2021 15:22

fwolter requested changes Jul 10, 2021

View reviewed changes

THKDev added 2 commits July 10, 2021 18:02

Revert "[fronius] fixed checkstyle error; removed some warnings (eg. …

62c735b

…StringUtils)" This reverts commit 73065c6 Signed-off-by: THKDev <THKDev@users.noreply.github.com>

[fronius] fixed checkstyle error

01988c2

Signed-off-by: THKDev <THKDev@users.noreply.github.com>

fwolter reviewed Jul 10, 2021

View reviewed changes

[fronius] removed org.apache.commons.lang3.StringUtils

52d80ad

Signed-off-by: THKDev <THKDev@users.noreply.github.com>

fwolter added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Jul 10, 2021

fwolter approved these changes Jul 11, 2021

View reviewed changes

fwolter merged commit 1e8be24 into openhab:main Jul 11, 2021

fwolter added the enhancement An enhancement or new feature for an existing add-on label Jul 11, 2021

fwolter added this to the 3.2 milestone Jul 11, 2021

jimtng added a commit to jimtng/openhab-addons that referenced this pull request Mar 1, 2022

[fronius] Document the recently added inverter power and soc channels

f957b82

This was added in openhab#10757 but the documentation was not updated. Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

jimtng added a commit to jimtng/openhab-addons that referenced this pull request Mar 1, 2022

[fronius] Document the recently added inverter power and soc channels

8a6ccb8

This was added in openhab#10757 but the documentation was not updated. Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

jimtng mentioned this pull request Mar 1, 2022

[fronius] Add the new channels to the documentation #12394

Merged

jimtng added a commit to jimtng/openhab-addons that referenced this pull request Mar 2, 2022

[fronius] Document the recently added inverter power and soc channels

cc1b939

This was added in openhab#10757 but the documentation was not updated. Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fronius] Added inverter power, battery state of charge and PV solar yield #10757

[fronius] Added inverter power, battery state of charge and PV solar yield #10757

THKDev commented May 24, 2021

fwolter commented Jun 5, 2021

trokohl commented Jun 5, 2021

fwolter Jun 5, 2021

fwolter Jun 5, 2021

fwolter Jun 5, 2021

THKDev Jun 24, 2021

fwolter Jun 24, 2021

THKDev Jun 25, 2021

fwolter Jun 25, 2021

THKDev Jun 26, 2021

fwolter commented Jun 30, 2021

fwolter left a comment

fwolter Jul 10, 2021

fwolter Jul 10, 2021

fwolter Jul 10, 2021

fwolter Jul 10, 2021

fwolter Jul 10, 2021

fwolter left a comment

	public static final String PowerFlowpPv = "powerflowchannelppv";
	public static final String POWER_FLOW_P_PV = "powerflowchannelppv";

	<state pattern="%.2f W" readOnly="true"></state>
	<state pattern="%.2f %unit%" readOnly="true"></state>

	@Nullable
	public Integer deviceId;
	public @Nullable Integer deviceId;


		import org.apache.commons.lang3.StringUtils;

[fronius] Added inverter power, battery state of charge and PV solar yield #10757

[fronius] Added inverter power, battery state of charge and PV solar yield #10757

Conversation

THKDev commented May 24, 2021

fwolter commented Jun 5, 2021

trokohl commented Jun 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fwolter commented Jun 30, 2021

fwolter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fwolter left a comment

Choose a reason for hiding this comment