Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

adds kW·h as Unit #5346

Closed
wants to merge 1 commit into from
Closed

adds kW·h as Unit #5346

wants to merge 1 commit into from

Conversation

J-N-K
Copy link
Contributor

@J-N-K J-N-K commented Apr 3, 2018

fixes #5341

Signed-off-by: Jan N. Klug [email protected]

@J-N-K
Copy link
Contributor Author

J-N-K commented Apr 3, 2018

Travis fails in unrelkated JSON storage test.

@J-N-K
Copy link
Contributor Author

J-N-K commented Apr 4, 2018

Retriggering build

@J-N-K J-N-K closed this Apr 4, 2018
@J-N-K J-N-K reopened this Apr 4, 2018
@J-N-K
Copy link
Contributor Author

J-N-K commented Apr 4, 2018

I don‘t understand why so many builds fail lately. This is clearly unrelated, unfortunately before the core tests.

@maggu2810
Copy link
Contributor

I assume this error is raised because of #5318.
NPEs in tests are not catched anymore.
IMHO this change is correct but it identify that some tests need more love (e.g. the related one seems to be very timing dependent).

I assume this one (the JSON storage one) will be fixed by #5353

@J-N-K
Copy link
Contributor Author

J-N-K commented Apr 5, 2018

retriggering build after tests are finally fixed

@J-N-K J-N-K closed this Apr 5, 2018
@J-N-K J-N-K reopened this Apr 5, 2018
fixes #5341

Signed-off-by: Jan N. Klug <[email protected]>
Copy link
Contributor

@htreu htreu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my inline comments. Its all about using predefined conversion factors.

@@ -122,13 +123,19 @@ public String getName() {
public static final Unit<Time> WEEK = addUnit(Units.WEEK);
public static final Unit<Time> YEAR = addUnit(Units.YEAR);
public static final Unit<Volume> LITRE = addUnit(Units.LITRE);
public static final Unit<Energy> WATT_SECOND = addUnit(new ProductUnit<Energy>(Units.WATT.multiply(Units.SECOND)));
public static final Unit<Energy> WATT_HOUR = addUnit(WATT_SECOND.multiply(3600));
public static final Unit<Energy> KILOWATT_HOUR = addUnit(WATT_HOUR.multiply(1000));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to use the KILO prefix here:

import static org.eclipse.smarthome.core.library.unit.MetricPrefix.KILO;

// and

public static final Unit<Energy> KILOWATT_HOUR = addUnit(KILO(WATT_HOUR));

@@ -122,13 +123,19 @@ public String getName() {
public static final Unit<Time> WEEK = addUnit(Units.WEEK);
public static final Unit<Time> YEAR = addUnit(Units.YEAR);
public static final Unit<Volume> LITRE = addUnit(Units.LITRE);
public static final Unit<Energy> WATT_SECOND = addUnit(new ProductUnit<Energy>(Units.WATT.multiply(Units.SECOND)));
public static final Unit<Energy> WATT_HOUR = addUnit(WATT_SECOND.multiply(3600));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure but maybe we should define WATT_HOUR also by ProductUnit with Units.HOUR. It already has the factor defined.

@J-N-K
Copy link
Contributor Author

J-N-K commented Apr 6, 2018

done. and integrated in #5336 to avoid merge conflicts

@J-N-K J-N-K closed this Apr 6, 2018
@J-N-K J-N-K deleted the add-kwh-unit branch August 21, 2018 09:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kWh are not recognized as Energy values
3 participants