[jdbc] Return QuantityTypes for number items with dimension

[cweitkamp]

• Return QuantityTypes for number items with dimension

This implementation uses the same approach like introduces for RRD4J in #8866 and #8938. But I am seeing some issue with Items of type Number:Dimensionless.

In general it would be possible to extend the database structure by adding a column for the unit for number items. This will need a little bit more effort.

@mhilbush May I ask you to chime in and help testing?

Related to openhab/openhab-core#1954

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

---

Depends on #9445

[mhilbush]

@mhilbush May I ask you to chime in and help testing?

Yes, certainly. This is for post 3.0, correct?

[cweitkamp]

Yes, certainly. This is for post 3.0, correct?

Thank you. Yes, my plan is to introduce this feature after OH 3 release.

[cweitkamp]

But I am seeing some issue with Items of type Number:Dimensionless.

I added a check which excludes all value having base unit ONE from being converted before storing them. This results in nice values again.

[cweitkamp]

I am running this version for nearly two weeks now and it seems to be stable. Feel free to do some additional testing or start review. Thanks.

[mhilbush]

One quick comment... If we're upgrading the mysql jdbc driver version to 8.0.13, this log message should be changed, too.

/~https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.persistence.jdbc/src/main/java/org/openhab/persistence/jdbc/internal/JdbcConfiguration.java#L306

[mhilbush]

And, unrelated to your PR, this line could be made debug.

[cweitkamp]

If we're upgrading the mysql jdbc driver version to 8.0.13, this log message should be changed, too.

Not sure if we have to do it, because the message shows information about the minimum required version. I thought about that too when I increased the MariaDB connector version but decided to not change it.

[mhilbush]

Not sure if we have to do it, because the message shows information about the minimum required version.

But when I installed 5.1.37, it didn't work. Then I looked in the pom and saw the 8.0.13 dependency. So I installed 8.0.13 and it started to work. I'll try again to see what it's complaining about.

[mhilbush]

Maybe I missing something, but none of the 5.1.* or 6.0.* versions I've tried will work for me. Only the 8.0.x versions work.

The older versions all result in this.

2020-12-29 11:28:58.203 [DEBUG] [b.persistence.jdbc.internal.JdbcPersistenceService] - JDBC::activate: persistence service activated
2020-12-29 11:28:58.203 [DEBUG] [b.persistence.jdbc.internal.JdbcPersistenceService] - JDBC::updateConfig
2020-12-29 11:28:58.203 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::JdbcConfiguration
2020-12-29 11:28:58.204 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: configuration size = 11
2020-12-29 11:28:58.204 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: user=XXXXX
2020-12-29 11:28:58.204 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: password exists? true
2020-12-29 11:28:58.204 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: url=jdbc:mysql://XXXXXXXXX:3306/openhab3?serverTimezone=America/New_York
2020-12-29 11:28:58.204 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: found serviceName = 'mysql'
2020-12-29 11:28:58.204 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: Init Data Access Object Class: 'org.openhab.persistence.jdbc.db.JdbcMysqlDAO'
2020-12-29 11:28:58.204 [DEBUG] [org.openhab.persistence.jdbc.db.JdbcBaseDAO       ] - JDBC::initSqlTypes: Initialize the type array
2020-12-29 11:28:58.205 [DEBUG] [org.openhab.persistence.jdbc.db.JdbcBaseDAO       ] - JDBC::initSqlQueries: 'JdbcMysqlDAO'
2020-12-29 11:28:58.205 [DEBUG] [org.openhab.persistence.jdbc.db.JdbcMysqlDAO      ] - JDBC::initSqlTypes: Initialize the type array
2020-12-29 11:28:58.205 [DEBUG] [org.openhab.persistence.jdbc.db.JdbcMysqlDAO      ] - JDBC::initSqlQueries: 'JdbcMysqlDAO'
2020-12-29 11:28:58.205 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: dBDAO ClassName=org.openhab.persistence.jdbc.db.JdbcMysqlDAO
2020-12-29 11:28:58.205 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: tableUseRealItemNames=true
2020-12-29 11:28:58.205 [DEBUG] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: enableLogTime false
2020-12-29 11:28:58.206 [ERROR] [penhab.persistence.jdbc.internal.JdbcConfiguration] - JDBC::updateConfig: could NOT load JDBC-driverClassName or JDBC-dataSourceClassName. ClassNotFoundException: 'com.mysql.jdbc.Driver cannot be found by org.openhab.persistence.jdbc_3.1.0.202012291431'
2020-12-29 11:28:58.206 [WARN ] [penhab.persistence.jdbc.internal.JdbcConfiguration] -

	!!!
	To avoid this error, place an appropriate JDBC driver file for serviceName 'mysql' in addons directory.
	Copy missing JDBC-Driver-jar to your openHab/addons Folder.
	!!!
	DOWNLOAD:
	MySQL:     version >= 5.1.36 from             https://mvnrepository.com/artifact/mysql/mysql-connector-java

I realize this is unrelated to your changes, so let me see if I can get it sorted out.

[cweitkamp]

tbh I don't know which version is missing for MySQL and when it has been changed to the used one. Maybe you can find an answer on the old openhab1-addons repository.

[mhilbush]

In openhab1-addons, I saw where the mysql persistence service was updated to use the 8.0.13 driver back in November 2018, and the mysql version in the jdbc persistence service was updated to use 8.0 in December 2018. But the jdbc service's README still says it was tested with the 5.1.39 driver version. I dunno. In any event, 8.0.13 works so that's what I'm using.

Question on averageSince and deltaSince. I've never used those before, and I see they currently return a DecimalType. As compared to maximumSince and minimumSince, both of which return an item (which is correctly representaed as a QuantityType). Do you know why averageSince and deltaSince are treated differently from maximumSince and minimumSince?

Even the documentation is confusing, as it doesn't say anything about the return type of averageSince and deltaSince.

Further down in the README here, it shows an example using temperature where it actually includes the units in answer. 🤔

[cweitkamp]

I agree, the docs might be confusing in the example.

Beside others minimumSince and maximumSince return a HistoricItem which is kind of wrapper for the historic state to transport the timestamp as well.

averageSince and deltaSince are performing an internal calculation (in OHC - see PersistenceExtensions for more details). Thus all historic values are converted to DecimalType before doing the math. This is not the best solution when dealing with UoM values. I already filed openhab/openhab-core#1954 for it and started testing / working on it but as a requirement to solve it all - or nearly all - persistence add-ons have to be capable of returning QuantityType values. This now is possible.

[mhilbush]

Ok, that's what I figured. And changing it will be a breaking change, right? Since existing rules depend on those functions always returning a DecimalType.

Other than that bit of confusion on my part, everything I've seen looks good.

[cweitkamp]

And changing it will be a breaking change, right?

Changing the return value is a breaking change.

[mhilbush]

I ran into another issue, but I don't think it's related to your changes because it also happens when I use an item defined as just a plain Number.

The functions varianceSince and deviationSince (which is derived from varianceSince) are returning null.

I know this call works, because it's also used to get averageSince and that function is working correctly.

But here, does it need to be reinitialized (or can it just use result to do the iteration)?

Or, is there something else I'm missing?

[mhilbush]

I added the line it = result.iterator(); above line 400 in PersistenceExtensions.java. The functions varianceSince and deviationSince produced results instead of returning null. If you think that's the correct way to fix it, I can submit a PR for core.

[cweitkamp]

Thanks for the analysis. What makes me curious is that my unit test in PersistenceExtensionsTest are working. Might they be wrong too?

[mhilbush]

Hmm. That's really weird.

Am I right that by the time it gets to line 400, the iterator it is at the end of the iteration as a result of calling internalAverageSince?

[mhilbush]

Doesn't there need to be an @Test in front of public void testVarianceSince for it to execute the test?

[cweitkamp]

🤦 Guess what ... The methods testVarianceSince() and testDeviationSince() are not annotated by @Test and thus not executed.

// EDIT: You figured it a second before me. And your suggestion is working. 👍 PR would be appreciated. Thanks.

[mhilbush]

Sure. Will do.

Shall I also add the annotations to the tests?

[cweitkamp]

Of course. This will validate that you are right. 😉

[mhilbush]

openhab/openhab-core#2036

Tests now run and succeed.

[cweitkamp]

@openhab/add-ons-maintainers Ready for review. Thanks.