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

Adapt to latest OpenHab 3.1.0-SNAPSHOT #7

Merged
merged 6 commits into from
Aug 8, 2021
Merged

Conversation

t-8ch
Copy link
Contributor

@t-8ch t-8ch commented May 16, 2021

Only compile-tested, as I don't have the hardware yet.

The eclipse project files should not be needed anymore, but I am not completely sure about that, as I'm not using eclipse.

Is there a reason in-lining the jmbus sources instead of using it from maven central? I have another (compile-only-tested) patch that replaces the bundled source.

@MikeTheTux
Copy link
Contributor

MikeTheTux commented Jun 2, 2021

Is there a reason in-lining the jmbus sources instead of using it from maven central?

The in-lined jmbus version includes a small extension for the access to the RAW frame

@t-8ch
Copy link
Contributor Author

t-8ch commented Jun 2, 2021

The in-lined jmbus version includes a small extension for the access to the RAW frame

Then it should be possible to only ship that extension with the binding and pull the rest of the binding from maven.
The build system will unpack the jar of jmbus into the jar of the binding, so the end result is the same as before.
This now also part of this PR.

@MikeTheTux
Copy link
Contributor

hey,

I took your PR, compiled it and integrated it in OH3.1.0.
It did not work.
After performing the following changes it is up and running again:

  • reverted changes in WMBusDevice.java (added getRawData() again)
  • added DataRecord.java again, overriding the file from the maven package and thereby introducing getRawData()
  • re-added jrxtx from smartmeter binding (original jrxtx v1.0.1 is not functional)

With these modifications, the WMBus Binding operates again in my OH3.1.0 installation.

Here is the resulting package:
org.openhab.binding.wmbus-3.1.0-SNAPSHOT-20210731.zip

@t-8ch
Copy link
Contributor Author

t-8ch commented Aug 1, 2021

How did it not work?

@MikeTheTux
Copy link
Contributor

The binding loaded but the Thing didn't went ONLINE. It was not able to open the serial connection and remained OFFLINE.
I'm also using the Smartmeter binding. Can there be a conflict when using different jrxtx implementations in parallel!?

@t-8ch
Copy link
Contributor Author

t-8ch commented Aug 2, 2021

If it compiled, than I think the changes you did to WMBusDevice.java and DataRecord.java should not be needed.
Can you try to also get it to work with only the new jrxtx?
(I don't have a device compatible with this binding at the moment)

@MikeTheTux
Copy link
Contributor

MikeTheTux commented Aug 3, 2021

getRawData() is used to override getDataValue().

@Override
public byte[] getDataValue() {
   return getRawData();
}

I guess this is the reason why it also compiles even without getRawData().

@t-8ch
Copy link
Contributor Author

t-8ch commented Aug 3, 2021

Did you test it without the changes to WMBusDevice and DataRecord but the fixed version of jrxtx?
I'm still not sure whether these changes should make a difference.

Nothing ever really calls the getRawData methods.
Only getDataValues and that still returns the raw data.

@MikeTheTux
Copy link
Contributor

I was wondering the same ...

I'm using now this PR and only reverted jrxtx to the original version.
My watermeter is online. I will report if it works stable ...

@t-8ch
Copy link
Contributor Author

t-8ch commented Aug 5, 2021

I dropped the patch that remove the bundled jmbus and jrxtx libraries.
Now the PR does again what it describes to do in it's title.
@MikeTheTux Could you validate that, too?

@MikeTheTux
Copy link
Contributor

MikeTheTux commented Aug 5, 2021

@MikeTheTux Could you validate that, too?

Smoke test positive.
I will report if it works stable ...

@MikeTheTux
Copy link
Contributor

... the latest version seems to operate stable for my use case 🙂

@t-8ch
Copy link
Contributor Author

t-8ch commented Aug 7, 2021 via email

@pokerazor pokerazor merged commit c0f18cd into KuguHome:master Aug 8, 2021
@pokerazor
Copy link

Hey @MikeTheTux @t-8ch thanks for your amazing work!

As you probably have noticed, lately this repo and the Thread on the forum has become quite silent, unfortunately. The reason is, that his binding (which had begun as a private project of myself and then became a part of the company I work for, KUGU Home GmbH) is not currently in the focus a lot anymore, as the business changed a bit.

However, we're still very interested in it and stabilizing it. So it's fortunate that you as contributors turned up. Would you be interested to have a chat in the coming weeks, to talk about ideas for next steps? I could send you a Microsoft Teams invite?

@t-8ch
Copy link
Contributor Author

t-8ch commented Aug 8, 2021

@pokerazor Sure, let's have a chat. You can use the email address from my commit on GitHub.

@MikeTheTux
Copy link
Contributor

@pokerazor Sure, let's have a chat. You can use the email address from my commit on GitHub.

same for me

@MikeTheTux
Copy link
Contributor

There is also a wish from openhab to contribute this as an official binding.
See new-binding-wireless-m-bus-techem-heat-cost-allocators#101 .

@t-8ch
Copy link
Contributor Author

t-8ch commented Aug 9, 2021

This would also have been my proposal to be honest:

  • No need for new developer-provided builds for users.
  • No need to keep up to date with internal refactorings of the addon system (like this very PR is doing)
  • No need for specific installation instructions.
  • Reuse of the modified jrxtx library from the smartmeter binding.
  • Fallback maintainers.

I would be willing to help with the contribution.
(FYI the contribution for my other binding went very smoothly)

I'm still trying to get hold of a reasonably priced adapter stick, maybe @pokerazor has a spare one?
Otherwise I'll probably buy a retail one.

@MikeTheTux
Copy link
Contributor

... the latest version seems to operate stable for my use case 🙂

For reference, this is my resulting binding from August'21 that I'm still using:

@t-8ch
Copy link
Contributor Author

t-8ch commented Oct 22, 2021

@pokerazor Any news about the planned talk?
If you give me a go-ahead I could also try submit it upstream on my own (maybe in collaboration with @MikeTheTux )

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.

3 participants