[shelly] Adapt shelly v1 handler to californium 4.0.0-M6

[holgerfriedrich]

This is the next try to get Shelly v1 running with californium 4.0.0-M6.
May resolve #20727.

• Revert "[shelly] Adapt shelly v1 handler to californium 4.0.0-M6 ([shelly] Adapt shelly v1 handler to californium 4.0.0-M6 #20704)"
  This reverts commit d90d079.
• Try to parse the contents directly.

[holgerfriedrich]

@ErikDB87 FYI

[holgerfriedrich]

just to document why I use encode() to get data bytes:
https://javadoc.io/doc/org.eclipse.californium/californium-core/latest/org/eclipse/californium/core/coap/Option.html#encode()`

Since: 4.0 (similar to previous getValue(), but reflects, that it is rather a conversion than just a get.)

[Nadahar]

@holgerfriedrich As I've mentioned in the issue, I can't understand that this is necessary. I've done a tiny tweak to your previous fix and made a build that I've posted on the forum, hoping that somebody can confirm that it works. I can't quite understand how it can't.

[Nadahar]

I've created #20729, and I think there's no need for further work on this.

[lsiepel]

As per above comment

[Nadahar]

@holgerfriedrich Just for the record: It doesn't look like there are problems caused by the changes to Shelly1CoapHandler (with extracting the types), but with the changes made to Shelly1CoapServer with the registration of the "custom options".

edit: Looking at your latest changes, this might in fact be related. The custom options must be registered to receive the "correct types", which is what starts all this. So, by finding other ways to read the data, we might avoid the whole registration..?

[holgerfriedrich]

@Nadahar I have created #20735 to avoid global registration.
It is maybe a matter of style how we want to proceed. Hopefully one of the PRs works.

[markus7017]

Question: Why to we upgrade to a milestone build? M6 is not an official release, but breaks compatibility to existing bindings

[holgerfriedrich]

Question: Why to we upgrade to a milestone build? M6 is not an official release, but breaks compatibility to existing bindings

Valid question. The point is we were on an earlier milestone release (M2) before, for quite a while now. I cannot remember the reason why I did an upgrade to a milestone. What I remember is that the stable version we used before had kind of a hacky replacement for ecdsa lib which caused warnings during compilation.

Of course we can go back to M2 in core. I'd assume that the compilation problem we experienced will be back with the stable release (whenever it is). tradfri change was minimal (one class to be renamed.

Do we have reason to believe that M6 causes other issues besides my obviously wrong fix for the compilation problem after the upgrade?

[markus7017]

Hmm, now I have to deal with 3 PRs and bring them together before I could give it a try
I'm working on PR #20396, but there is also #20728 and #20735 - at least #20728 and #20735 should go into one

[markus7017]

@Nadahar would it make sense to revert the rebase for #20396 and finish PR, then do the rebase and move on with the Coap topic?

[markus7017]

@holgerfriedrich We should consider that this breakss compatibility to OH 5.1 (not resolving the dependency). Did you checked the Tradfi binding, this also uses Californium

[Nadahar]

Do we have reason to believe that M6 causes other issues besides my obviously wrong fix for the compilation problem after the upgrade?

What I see as perhaps the biggest problem is that test/dev JARs won't be compatible across the divide. Shelly is undergoing some major refactoring, and there's a great need for testing by users because you need the devices to test. Each change must essentially be build twice, one for "Californium M2" and one for M6. Changing core checkout takes ages for me, so that will mean several hours just for making one build of each.

These changes will most likely be back in the final release, so if we might as well "eat them now", but it is very inconvenient. If we could come up with some code that works for both versions, it would make everything much easier.

[markus7017]

These changes will most likely be back in the final release, so if we might as well "eat them now", but it is very inconvenient. If we could come up with some code that works for both versions, it would make everything much easier.

I think there is no way, because of structural changes and 5.1 does not provide the M6 dependency. I see no benefit to make the move now, it just creates pain while not fixing specific problems. I'm blocked and we can't finish #20396

I prefer going back to M2, finishing #20396
and maybe it's a better idea to move to M6 once OH 5.2 has been released.

[Nadahar]

would it make sense to revert the rebase for #20396 and finish PR, then do the rebase and move on with the Coap topic?

It's difficult to know what is the best way. The problem for me is that I also work on core, so I have to have an up-to-date core checked out. That means that the updated dependency will be used regardless of what the openhab-addons repo is "rebased against", so I will get compilation errors and be unable to build. This is how I discovered this in the first place, I rebased my core PR, and suddenly Shelly had compilation errors.

It's not "difficult" to check out a different version of core, but because Eclipse must be "playing along", it means that all of core must be built - multiple times, because when one bundle is built, Eclipse will also rebuild every bundle that depends on it - and this goes on almost forever before it's finished. In addition, there are problems with certain core bundles that result in endless build loops, and I must manually "break" those loops, so I can't just leave the computer alone while it does this - I must actually monitor the process to some extent.

So, to the degree I should be involved, it will kind of "have to" be working with latest core. Everything would be much easier if we could find a solution that would work against both M2 and M6...

[Nadahar]

I prefer going back to M2, finishing #20396
and maybe it's a better idea to move to M6 once OH 5.2 has been released.

We will get the same situation no matter when it's done. If it's bumped to M6 just after 5.2 has been released, it means that all snapshot builds of 5.3 will be incompatible with 5.2. In that sense, since we're not too far away from 5.2 release, now isn't such a bad time.

I'm still hoping that one of @holgerfriedrich's approaches will work with both versions. If we can avoid the registration, I think the "new code" will also work with M2.

[Nadahar]

I've looked at this a bit now, and this is what I'd call very bad practiced by Californium. They have defined Option subclasses in 4.0 only, before that they don't exist. Then they remove the "general getters" also in 4.0 (somewhere before M6), which means that there is no "grace period" where things are deprecated but still works. This makes it very hard for consumers that have to balance the puzzle of a complex dependency tree and still make things work.

There are OptionDefinition classes that allow extracting the information we want that have existed since 3.8, but they depend on getValue() as input, which has also been removed from Option in M6 (it's now in OpaqueOption, but this class doesn't exist M2. This is actually very frustrating and could so easily have been avoided with just a little bit of care.

[lsiepel]

Can we get it all working with m6 this weekend?
If not I opt to revert to m2 and develop a fix for m6 In the background. This would enable us to finish multiple Shelly PR’s who already struggle a lot to move forward.

[lsiepel]

As we do not seem to have a proper fix, I’d like to revert m6 immediately.
Please advice the PR to revert as we did several.