[entsoe] Initial contribution #17416

jmelhus · 2024-09-14T09:50:40Z

No description provided.

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

jmelhus · 2024-09-14T09:55:06Z

Please add label new binding, i do not seem to have the necessary rights to do so.

openhab-bot · 2024-09-14T09:59:16Z

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

https://community.openhab.org/t/entsoe-binding-4-0-0-5-0-0/149699/50

lsiepel · 2024-09-14T10:13:23Z

Thank you for creating this PR to contribute this binding.
To manage expectations; review capacity is somewhat limited so don’t expect an immediate full review. To speed up the process you can already perform a self-review by checking this list: https://github.com/openhab/openhab-addons/wiki/Review-Checklist

Have not looked at this PR yet, it may be of very good quality. But common review comments for new bindings are about thing structure (xml), naming convention and log levels. So you might double check those areas.

It will also help to have a summary OP, with testable jar etc

jlaur

Many thanks for contributing this new binding! 👍

I have had a very quick initial look, please find my comments below. Also I see some issues with naming convention compliance and null annotations, please have a look at the SAT report and compiler warnings.

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

....openhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/entsoeHandler.java

...e/src/main/java/org/openhab/binding/entsoe/internal/exception/entsoeUnexpectedException.java

bundles/org.openhab.binding.entsoe/src/main/resources/OH-INF/i18n/entsoe.properties

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

...enhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/client/EntsoData.java

...binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/entsoeBindingConstants.java

jlaur

I have added a few additional comments.

bundles/org.openhab.binding.entsoe/README.md

bundles/org.openhab.binding.entsoe/pom.xml

bundles/org.openhab.binding.entsoe/README.md

....openhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/client/Client.java

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

…ion.java Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

…java Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

jmelhus · 2024-09-15T16:45:56Z

@jlaur , thanks for the review. I have implemented the changes. For the httpclient i suggest to implement that in next release.

jlaur

Thanks for quickly addressing review comments. I have added a few more on top of the few remaining comments not yet addressed.

I do not expect to find anything major going forward, so probably we are approaching a final round.

There are also a few compiler warnings left to address.

bundles/org.openhab.binding.entsoe/README.md

....openhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/client/Client.java

bundles/org.openhab.binding.entsoe/README.md

....openhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/EntsoeHandler.java

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

jmelhus · 2024-09-16T21:08:56Z

@jlaur Thanks for the second review, i have looked into them and implemented the changes.

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

jlaur

I have added a few more comments - I consider this my last round. I hope to give the binding a test run this weekend.

Please note that there are also some unresolved/uncommented comments from the first two rounds. You might have to click "Load more..." to see hidden conversations.

bundles/org.openhab.binding.entsoe/README.md

....openhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/EntsoeHandler.java

jlaur · 2024-09-20T11:03:22Z

....openhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/EntsoeHandler.java

+    }
+
+    private State getPriceState(BigDecimal kwhPrice) {
+        if (baseUnit == null || fromUnit == null) {


I believe neither baseUnit or fromUnit is never null since they are set in lines 69-70 and never changed:

private Unit<Currency> baseUnit = CurrencyUnits.BASE_CURRENCY; private Unit<Currency> fromUnit = new CurrencyUnit(EntsoeBindingConstants.ENTSOE_CURRENCY, null);

....openhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/EntsoeHandler.java

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

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

jlaur

@jmelhus - I'm wondering if you are in GitHub self-review mode and did not publish any of your own comments yet? Or did you actually not respond to any of my comments?

jlaur · 2024-09-23T19:31:52Z

....openhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/EntsoeHandler.java

+
+    private void refreshPrices() {
+        if (!isLinked(EntsoeBindingConstants.CHANNEL_SPOT_PRICE)) {
+            logger.debug("Channel {} is not linked, cant update channel", EntsoeBindingConstants.CHANNEL_SPOT_PRICE);


Actually we can, we just don't need to. It's an optimization.

Suggested change

logger.debug("Channel {} is not linked, cant update channel", EntsoeBindingConstants.CHANNEL_SPOT_PRICE);

logger.debug("Channel {} is not linked, skipping channel update", EntsoeBindingConstants.CHANNEL_SPOT_PRICE);

jlaur · 2024-09-23T19:33:21Z

....openhab.binding.entsoe/src/main/java/org/openhab/binding/entsoe/internal/EntsoeHandler.java

+            } catch (EntsoeResponseException | EntsoeConfigurationException e) {
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());


Suggested change

} catch (EntsoeResponseException | EntsoeConfigurationException e) {

updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());

} catch (EntsoeResponseException e) {

updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());

} catch (EntsoeConfigurationException e) {

updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getMessage());

jlaur · 2024-09-23T19:34:47Z

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

+
+	<thing-type id="day-ahead">
+
+		<label>Day ahead prices</label>


Suggested change

<label>Day ahead prices</label>

<label>Day-ahead prices</label>

initial contribution

bbaeec8

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

jmelhus requested a review from a team as a code owner September 14, 2024 09:50

lsiepel added the new binding If someone has started to work on a binding. For a new binding PR. label Sep 14, 2024

jlaur requested changes Sep 14, 2024

View reviewed changes

jlaur changed the title ~~[entsoe] initial contribution~~ [entsoe] Initial contribution Sep 14, 2024

jlaur reviewed Sep 14, 2024

View reviewed changes

jmelhus and others added 8 commits September 15, 2024 18:21

Implemented changes based on review feedback

020adbc

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

Rename entsoeConfigurationException.java to EntsoeConfigurationExcept…

f5455bc

…ion.java Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

Rename entsoeResponseException.java to EntsoeResponseException.java

721639b

Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

Rename entsoeResponseMapException.java to EntsoeResponseMapException.…

142f8e3

…java Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

Rename entsoeBindingConstants.java to EntsoeBindingConstants.java

6af9eb3

Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

Rename entsoeConfiguration.java to EntsoeConfiguration.java

f487d66

Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

Rename entsoeHandler.java to EntsoeHandler.java

f30a47b

Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

Rename entsoeHandlerFactory.java to EntsoeHandlerFactory.java

3255fd5

Signed-off-by: Jørgen Melhus <5099412+jmelhus@users.noreply.github.com>

jlaur requested changes Sep 15, 2024

View reviewed changes

jmelhus added 2 commits September 16, 2024 22:53

Implemented changes based on review feedback

bd6380f

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

Fix format violation

3252d99

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

Fix, add schedule refreshPrices to initialize method

5d0b959

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

jlaur added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Sep 18, 2024

jlaur requested changes Sep 20, 2024

View reviewed changes

jlaur reviewed Sep 21, 2024

View reviewed changes

bundles/org.openhab.binding.entsoe/src/main/resources/OH-INF/thing/thing-types.xml Outdated Show resolved Hide resolved

Implemented changes based on review feedback

78e4f8a

Signed-off-by: Jørgen Melhus <jmelhus@outlook.com>

jlaur reviewed Sep 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[entsoe] Initial contribution #17416

[entsoe] Initial contribution #17416

jmelhus commented Sep 14, 2024

jmelhus commented Sep 14, 2024

openhab-bot commented Sep 14, 2024

lsiepel commented Sep 14, 2024

jlaur left a comment

jlaur left a comment

jmelhus commented Sep 15, 2024

jlaur left a comment •

edited

Loading

jmelhus commented Sep 16, 2024

jlaur left a comment

jlaur Sep 20, 2024

jlaur left a comment

jlaur Sep 23, 2024

jlaur Sep 23, 2024

jlaur Sep 23, 2024

	logger.debug("Channel {} is not linked, cant update channel", EntsoeBindingConstants.CHANNEL_SPOT_PRICE);
	logger.debug("Channel {} is not linked, skipping channel update", EntsoeBindingConstants.CHANNEL_SPOT_PRICE);

		} catch (EntsoeResponseException \| EntsoeConfigurationException e) {
		updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());

	<label>Day ahead prices</label>
	<label>Day-ahead prices</label>

[entsoe] Initial contribution #17416

Are you sure you want to change the base?

[entsoe] Initial contribution #17416

Conversation

jmelhus commented Sep 14, 2024

jmelhus commented Sep 14, 2024

openhab-bot commented Sep 14, 2024

lsiepel commented Sep 14, 2024

jlaur left a comment

Choose a reason for hiding this comment

jlaur left a comment

Choose a reason for hiding this comment

jmelhus commented Sep 15, 2024

jlaur left a comment • edited Loading

Choose a reason for hiding this comment

jmelhus commented Sep 16, 2024

jlaur left a comment

Choose a reason for hiding this comment

jlaur Sep 20, 2024

Choose a reason for hiding this comment

jlaur left a comment

Choose a reason for hiding this comment

jlaur Sep 23, 2024

Choose a reason for hiding this comment

jlaur Sep 23, 2024

Choose a reason for hiding this comment

jlaur Sep 23, 2024

Choose a reason for hiding this comment

jlaur left a comment •

edited

Loading