Skip to content
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

Make DecimalType, QuantityType, PercentType accept lowercased exponent notation #3834

Merged
merged 3 commits into from
Oct 9, 2023

Conversation

jimtng
Copy link
Contributor

@jimtng jimtng commented Oct 7, 2023

@jimtng jimtng requested a review from a team as a code owner October 7, 2023 12:31
@wborn
Copy link
Member

wborn commented Oct 7, 2023

If this notation is to be supported I would also expect it to be supported by all the others types such as PercentType and QuantityType. There don't seem to be any unit tests for those that indicate it is supported.

@jimtng jimtng force-pushed the decimaltype-uppercase branch from b2d682b to e5773ab Compare October 7, 2023 14:09
@jimtng jimtng changed the title Make DecimalType recognize lowercased e notation Make DecimalType, QuantityType, PercentType accept lowercased exponent notation Oct 7, 2023
@jimtng jimtng force-pushed the decimaltype-uppercase branch from e5773ab to b8f7739 Compare October 7, 2023 14:13
…t notation

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
@jimtng jimtng force-pushed the decimaltype-uppercase branch from b8f7739 to b027633 Compare October 7, 2023 14:20
@jimtng jimtng force-pushed the decimaltype-uppercase branch 2 times, most recently from d574864 to c4bbd56 Compare October 7, 2023 22:15
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
@jimtng jimtng force-pushed the decimaltype-uppercase branch from c4bbd56 to ee1743d Compare October 7, 2023 22:26
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Copy link
Member

@wborn wborn left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!

@wborn wborn merged commit 304453e into openhab:main Oct 9, 2023
@wborn wborn added this to the 4.1 milestone Oct 9, 2023
@jimtng jimtng deleted the decimaltype-uppercase branch October 9, 2023 09:20
@J-N-K
Copy link
Member

J-N-K commented Oct 9, 2023

This breaks the KNX tests, see https://github.com/openhab/openhab-addons/actions/runs/6461008060/job/17539806529?pr=15723

  • 1 /s can't be parsed anymore
  • 1 Ω·m can't be parsed anymore

@jimtng
Copy link
Contributor Author

jimtng commented Oct 9, 2023

This is because the regex is too restrictive as I mentioned above. Perhaps we do need to change it to a broader pattern after all.

jimtng added a commit to jimtng/openhab-webui that referenced this pull request Aug 7, 2024
Core now accepts lowercased exponent since
openhab/openhab-core#3834

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
lolodomo pushed a commit to openhab/openhab-webui that referenced this pull request Aug 8, 2024
Core now accepts lowercased exponent since
openhab/openhab-core#3834

@mherwege would you mind verifying this please?

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DecimalType: Parsing floats with a lowercase "e" separating mantissa and exponent fails
3 participants