-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[energidataservice] Provide work-around for currency issues #16085
[energidataservice] Provide work-around for currency issues #16085
Conversation
Signed-off-by: Jacob Laursen <[email protected]>
75e459c
to
f1b67ea
Compare
return new QuantityType<>(price + " " + currency.getSymbol() + "/kWh"); | ||
} catch (IllegalArgumentException e) { | ||
logger.trace("Unable to create QuantityType, falling back to DecimalType", e); | ||
return new DecimalType(price); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also fall back to CurrencyUnits.BASE_CURRENCY.getSymbol()
, which is always available. Or check if the desired unit is available by CurrencyUnits.getInstance().getUnit(String s)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I'm now verifying the unit before trying to instantiate QuantityType
from it.
Additionally I will now fallback to currency code if there is no symbol for the currency, although my tests suggest that currency code as unit is not supported. Also I'm now using getSymbol()
on the returned Unit
rather than Currency.getSymbol(), because that is display unit for the Locale
and thus prone to failure (e.g. "$" vs. "US$").
I don't think using base currency is such a good idea, because that would mean explicitly providing a wrong unit. The end result is currently the same though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this only as a work-around to ensure reliable state updates in 4.1, would you agree? And do you see anything in all this that is worth creating an issue for in core?
We currently have the limitation that the UoM system know only one currency. When this will change, I would somehow expect:
- Always being able to update state for known currencies, even if there is no currency exchange service provided/installed - without risking
IllegalArgumentException
being thrown. With known currencies, all I mean all defined by ISO 4217. - In this case IMHO it would still be better to keep the source currency unit rather than overwriting by user configured currency unit without converting the amount. I know this contradicts 4.0 UoM because we cannot enforce "target unit", and I don't have a good proposal for tackling that. It's different, because we can always expect to be able to convert °C to °F, but we can't always expect to be able to convert DKK to EUR. And we cannot make the assumption that exchange rate is 100, because then prices will be plain misleading.
- Being able to update state with currency code as unit rather than symbol. When all currencies will become available and we are able to convert between them, symbols can become ambiguous as previously discussed.
Signed-off-by: Jacob Laursen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/using-the-new-currency-units-of-measurement-in-4-1/152338/17 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/using-the-new-currency-units-of-measurement-in-4-1/152338/22 |
…16085) * Provide work-around for currency issues * Verify unit before trying to instantiate QuantityType Signed-off-by: Jacob Laursen <[email protected]> Signed-off-by: Jørgen Austvik <[email protected]>
…16085) * Provide work-around for currency issues * Verify unit before trying to instantiate QuantityType Signed-off-by: Jacob Laursen <[email protected]>
This temporary work-around addresses some issues regarding currency support in openhab/openhab-core#3503 and #16070.
See #16070 (comment)
Issues:
LocaleBasedCurrencyProvider
) or configured fixed base currency is not DKK (forFixedCurrencyProvider
).The provided work-around will ensure that
Number
items can always be updated, andNumber:EnergyPrice
as well, although it cannot be guaranteed to be in the correct currency/unit.