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

[UoM] unit bar not working with prefix #6411

Closed
J-N-K opened this issue Oct 25, 2018 · 4 comments · Fixed by #6420
Closed

[UoM] unit bar not working with prefix #6411

J-N-K opened this issue Oct 25, 2018 · 4 comments · Fixed by #6420
Assignees
Labels

Comments

@J-N-K
Copy link
Contributor

J-N-K commented Oct 25, 2018

@Test 
public void UnithPaTest() {
    QuantityType<Pressure> hPa = new QuantityType<>(20, MetricPrefix.HECTO(SIUnits.PASCAL));
    assertEquals("20 hPa", hPa.toString());

    QuantityType<Pressure> hPa2 = new QuantityType<>(hPa.toString());
    assertEquals(hPa, hPa2);
}

This works as expected.

@Test
public void UnitmbarTest() {
    QuantityType<Pressure> mbar = new QuantityType<>(20, MetricPrefix.MILLI(SmartHomeUnits.BAR));
    assertEquals("20 mbar", mbar.toString());
  
    QuantityType<Pressure> mbar2 = new QuantityType<>(mbar.toString());
    assertEquals(mbar, mbar2);
}

This fails with

java.lang.IllegalArgumentException: mbar not recognized (in 20 mbar at index 3)
	at org.eclipse.smarthome.binding.onewire.internal.UtilTest.UnitmbarTest(UtilTest.java:73)
@J-N-K
Copy link
Contributor Author

J-N-K commented Oct 25, 2018

@htreu, do we have to add mbar in a similar way as kWh? If so, I can provide a PR for that. However, it seems a bit strange that prefixes work for Pascal and bar when the string is constructed but only work for Pascal when it comes to parsing exactly that string.

@htreu
Copy link
Contributor

htreu commented Oct 29, 2018

We use the SimpleUnitFormat to parse units and it only knows a fixed set of unit symbols. Every combination has to be registered upfront.
I would happily accept a PR for adding mbar as a separate unit but in the long run we should see what other parsers exists or if we should write one ourselves to handle this in a more generic way.

@J-N-K
Copy link
Contributor Author

J-N-K commented Oct 29, 2018

I already created one but the Build fails in my repository due to ConfigDiscoveryTest (seems unrelated).

@J-N-K J-N-K mentioned this issue Oct 29, 2018
@htreu
Copy link
Contributor

htreu commented Oct 29, 2018

It also fails on the PR, I will investigate.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants