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

Split dongles into separate bundles #159

Merged
merged 8 commits into from
Apr 3, 2018

Conversation

cdjackson
Copy link
Contributor

This splits each dongle into a separate bundles as per #47. Currently it's tested with the Telegesis dongle only but all dongles have been refactored.
The library jars are included in the PR at this stage so that it can be tested. These just need to be included into the TP.
There's also some tidying needed of the readme files etc.
Signed-off-by: Chris Jackson chris@cd-jackson.com

Signed-off-by: Chris Jackson <chris@cd-jackson.com>
Signed-off-by: Chris Jackson <chris@cd-jackson.com>
Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@kaikreuzer
Copy link
Member

I am a bit uncertain on the effects of this restructuring. So far we had this repo designed in a way that it includes the bundle in the repo root, so that it nicely blends into https://github.com/openhab/openhab-bundles. Did you check whether this aggregated build still works as expected?
Also, can it easily be imported in the IDE and used from there as before?
Furthermore, is there any impact on the openhab-docs project, which aggregates the documentation from the different repos?

@tomhoefer
Copy link
Contributor

My expectation would be to have at least separated bundles for:

  • ZSS CL
  • ZSS Ember
  • ZSS Telegesis
  • General OH Binding
  • OH Binding specific logic for Ember
  • OH Binding specific logic for Telegesis

@cdjackson cdjackson force-pushed the split_bundles branch 3 times, most recently from 5fb5f03 to 439019f Compare March 11, 2018 09:54
@kaikreuzer
Copy link
Member

it shouldn't matter too much if we point them at my existing repo, or the OH repo (right?).

Well, they are not pointing to any repo, my idea was to have it included in the same Maven reactor, so that the dependencies are found. As you are not using Tycho, but the Felix Bundle Plugin for the OSGi build, I am not at all certain anymore whether this can work.

Then in a month or so I can maybe look at sorting out the whole repo (ie autocoder) move so that everything can migrate across.

If this is realistic and you say that the core framework libs are rather stable already, I'd think the easiest way to move forward is to include the external bundles in the target platform (https://github.com/openhab/openhab-deps-repo). If you could tell me which version (1.0.7, I suppose?) should be used, I can take care of this part. All you would then have to change in this PR is to completely remove the lib folder(s) and the m2e natures from the projects.

Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@kaikreuzer
Copy link
Member

@cdjackson Short ping to make sure that you noticed my comment above.

@cdjackson
Copy link
Contributor Author

Sorry Kai, I didn't see a notification for this.

my idea was to have it included in the same Maven reactor, so that the dependencies are found.

Can you explain your idea - does it also work in Eclipse IDE?

If you could tell me which version (1.0.7, I suppose?)

I would probably update to 1.0.8 to pickup some small fixes in the XBee driver, but I'd like to understand your alternative idea if there's a better way...

@kaikreuzer
Copy link
Member

Can you explain your idea

Well, you do not need to care about dependencies, if all that you depend on is part of your own build - and that's what openhab-bundles is currently doing.

does it also work in Eclipse IDE?

Yes, it works if you have all the projects in your workspace (i.e. the same as having all in the Maven reactor).

I would probably update to 1.0.8

Ok, if you want me to add this to the target platform, just ping me once it is available.

Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@cdjackson
Copy link
Contributor Author

@kaikreuzer I've released 1.0.8 this afternoon and just retested the binding (also with ZB3 :) ). I've removed the dependency JAR, so if y'all are happy, then we can look to get this all merged.

Let me know if there's anything else you want me to do...

Thanks

@kaikreuzer
Copy link
Member

Thanks @cdjackson, I have created openhab/openhab-distro#674, which adds the libs to the target platform in version 1.0.8.

What is still missing in this PR is the removal of all Maven specific stuff - like m2e natures on projects, dependencies in the poms and definition of maven repo for zsmartsystems. None of that is needed as the resolution is done through p2 and Tycho.

@cdjackson
Copy link
Contributor Author

Thanks @kaikreuzer. I've updated the POM to remove all the dependencies. This works fine in the IDE, but somewhere maven is still picking up 1.0.23 and the build is failing. If I manually change the POM to 1.0.24 and build locally, it works fine.

@kaikreuzer
Copy link
Member

@cdjackson It is not just the poms, it is also the m2e nature on the projects that need to be removed as I said. Otherwise there are error markers on the project in the IDE.

Once you have adapted that, is it ok from your end to merge? I.e. is all code up to date, so that I can adapt the distro to pick up these separate bundles instead of the old single one?

Wrt the Maven build: Yes, I still have to update the dependencies reference in the parent pom - will do so soon.

@kaikreuzer
Copy link
Member

FTR: openhab/openhab-core#327

Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@cdjackson
Copy link
Contributor Author

Hopefully I've removed all the m2e bits from the project.

Yes, once you're happy with the build then it can be merged.

@kaikreuzer
Copy link
Member

Ok, perfect, many thanks! Even Jenkins is now happy 👍

@kaikreuzer kaikreuzer merged commit a0cdc91 into openhab:master Apr 3, 2018
@kaikreuzer
Copy link
Member

Will now try to update the distro accordingly...

@cdjackson
Copy link
Contributor Author

cdjackson commented Apr 3, 2018 via email

@kaikreuzer
Copy link
Member

Ok, I hope that openhab/openhab-distro#676 will make the Zigbee binding work smoothly in the distro - for openHAB, we are simply adding all the bundles, when the user wishes to install the binding. Not really ideal to have 8 bundles for a single binding, but you didn't like my 4-bundle-solution... 🙄

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.

None yet

3 participants