openhab / openhab1-addons

Add-ons for openHAB 1.x
Eclipse Public License 2.0
3.43k stars 1.69k forks source link

[dynamodb] Instructions for completing the validation for inclusion in the openHAB2 distribution #5334

Closed ssalonen closed 6 years ago

ssalonen commented 6 years ago

The Persistence docs say that:

Some openHAB 1 persistence services have not yet completed validation for inclusion in the distribution; however, they may indeed work properly under openHAB 2.

How to complete this validation in practice?

I have personally tested the version in PR #5333 quickly using openHAB2, and it seems to work fine... Is there more guidance on the testing? I wonder why so many persistence services have been left out from the distribution?

Should I as the dynamodb addon developer move this addon to use smarthome packages? I notice that all (?) persistence addons still rely on compat layer?

Context

Community discussion re missing persistence services https://community.openhab.org/t/amazon-dynamodb-persistence-binding-not-in-the-addons-download/17616/2?u=ssalonen

Also mentioned here: https://community.openhab.org/t/new-amazon-dynamodb-persistence-addon-alpha-experimental/9856

Your Environment

9037568 commented 6 years ago

I'm not clear on where these would migrate to. OH2 addons or ESH?

ssalonen commented 6 years ago

Thanks for the reply! I think I used a bit wrong wording... There are two topics as addon developer which are unclear to me:

From my perspective it would be OK to move this to even ESH, I think the licenses should be clear for the actual addon code. Naturally there are the dependencies in lib with different license.

9037568 commented 6 years ago

I don't know the answers to these questions. I think we need input from someone better connected, like @kaikreuzer or @watou.

kaikreuzer commented 6 years ago

Hi @ssalonen, I guess this is the info you are looking for: http://docs.openhab.org/developers/development/compatibilitylayer.html#how-to-add-a-successfully-tested-1x-add-on-to-the-distribution

ssalonen commented 6 years ago

@kaikreuzer thanks! So in order to "complete the validation" I should follow the steps to create

I'm still having a bit hard time to understand everything so bear with me :)

Adding a karaf feature seems simple enough, there are many persistence services having this already. However, I found only single openHAB1 persistence service with parameters defined with xml, jdbc. This addon has the parameters defined in ESH-INF/config/config.xml.

The documentation talks about creating ESH-INF/binding/binding.xml and references XML Structure for Binding Definitions in ESH docs. This was my problem before, binding is well documented but not so much persistence. So should I really create ESH-INF/binding/binding.xml even for persistence addons, or should I follow jdbc example and create ESH-INF/config/config.xml?

Taking jdbc as example, I can also see some other parameters under OSGI-INF, service.config.*. Should those be added? Original commit does not provide more information.

Sorry for the simple questions and if these are answered somewhere. Please just point the docs if the info is somewhere but I can't just find it...

ssalonen commented 6 years ago

I'm still lacking information, really appreciate any pointers where to find more information.

9037568 commented 6 years ago

Afraid I don't have any additional input on that. Perhaps @kaikreuzer or @sjka can share the knowledge.

ssalonen commented 6 years ago

Discussed also here: https://community.openhab.org/t/new-amazon-dynamodb-persistence-addon-alpha-experimental/9856

It's been now 23 days since last reply. I hope that there is a way to progress this, so it would be easier for the users to take the addon into use.

Still not clear what are the necessary steps to get the addon included in openHAB2 distribution, see my comments above.

kaikreuzer commented 6 years ago

binding is well documented but not so much persistence.

Yes, sorry, you are right. So the best advice is indeed to do it in the same way as it is done for JDBC:

ssalonen commented 6 years ago

Thank you very much! I think I have all I need for now. I will submit a PR for dynamodb persistence. With the experiences from that I can contribute and improve the docs.

ssalonen commented 6 years ago

Hmm, can't seem to get this to work. Actually, I also have trouble with JDBC.

This is how JDBC looks with openHAB2.2. I installed it from Paper UI.

image

So no sign of parameters even though they are clearly in the codebase. Perhaps due to ESH-INF missing in build.properties? Is this a bug?

The DynamoDB persistence I have been working on looks better (branch: https://github.com/ssalonen/openhab1-addons/commits/dynamodb-esh-compat ):

image

Problem 1

However, when I change the parameters, the PersistenceService is not deactivated/activated (can be easily confirmed using debugger, for example). Interestingly, I can see that the internal config cache (?) at userdata/config/org/openhab/persistence/dynamodb.config gets updated instantly.

Problem 2

When I restart the bundle with bundle:restart NNN, the binding gets deactivated/activated as it should. However, there is another issue: the Map<String, Object> config passed to activate does not have the actual configuration parameters, only some other properties. From the debugger:

{service.config.category=persistence, service.config.label=Amazon DynamoDB Persistence, component.name=org.openhab.persistence.dynamodb, component.id=228, service.config.description.uri=persistence:dynamodb}

I tried enabling verbose logging but no leads why OSGI is not working as expected... According to this article "Configurations can be updated dynamically. Without any extra effort, this will mean your component gets shot down and then recreated with the latest configuration data.".

What am I missing? Sorry to ping you again @kaikreuzer :)

ssalonen commented 6 years ago

Anyone? Really would value any hints to proceed.

ssalonen commented 6 years ago

Update (with openHAB 2.3 stable): it seems that the above Problems 1 & 2 were both caused by the OSGI configuration. Fixed by removing immediate="true" and configuration-policy="optional", leaving only:

<scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.2.0" activate="activate"
deactivate="deactivate"
name="org.openhab.persistence.dynamodb"
configuration-pid="org.openhab.dynamodb">

It seems that file configuration can be used as well

region="test-region-file"

However, it seems that all old keys (now removed but present before) are present in the ServiceEvent REGISTERED (checking the logs). Probably separate issue along the lines of https://github.com/openhab/openhab-distro/issues/396

UPDATE: Defining service.pid in OSGI configuration:

<property name="service.pid" type="String" value="org.openhab.dynamodb"/>

and referencing this pid from the configuration file

pid:org.openhab.dynamodb
region=file-reg

makes changes override the UI. And no more old keys present.

Seems good so far.

ssalonen commented 6 years ago

PR to update documentation to make the steps more clear: https://github.com/openhab/openhab-docs/pull/756

The DynamoDB related changes were merged in https://github.com/openhab/openhab1-addons/pull/5635

I think we can now close this issue, and follow up doc updates in https://github.com/openhab/openhab-docs/pull/756