[nanoleaf] Refactored code to use core features and more

[kaikreuzer]

This is a bigger refactoring bringing these (partially breaking) changes:

• System channel types are used where applicable
• Obsolete channels (such as power) were removed
• Some channel types were marked "advanced"
• "Tap" channels were converted to a trigger channel type providing a "system button" behavior
• Layout can now be requested by a console command
• Command options for effect channel are dynamically provided
• Log level has been reduced where appropriate
• HTTP request timeouts were reduced
• handleRemoval now returns quickly as expected
• Fixed hanging thread / infinite loop when requesting layouts for non-square panels
• Various other smaller enhancements and fixes
• Documentation has been adapted accordingly

Signed-off-by: Kai Kreuzer kai@openhab.org

[kaikreuzer]

@stefan-hoehn Would be great if you could also review & test with your setup.
As I had missed to provide review comments to the previous PR on time, I have covered all desired changes in this PR - it works quite well and has some new features like the dynamic effect options that are now automatically populated:

[stefan-hoehn]

First of all, Kai, thanks a lot for working on the binding. I really appreciate that.
I am amazed how much and what you have changed - definitely a huge step forward for the nanoleaf binding!

I am eager to review but I probably need to take some time to understand what you have done (though I am sure that there hardly will be any comment from my side as you are much more experienced that I am). How quickly do you need my review?

And second question: How would I actually try out the pull request? What is the right way to do that? Just do a "git clone" from your repo fork onto my machine and test it?

And third question: Are all chances backward compatible? In some cases I was hesitant to change something because I was afraid people would need to recreate things (or items) as at least in one case I know that a user has around 200 of them?

[kaikreuzer]

How quickly do you need my review?

It is enough if you roughly scan through it - I just wanted to make sure to keep you in the loop of code changes as you are actively working on it as well. If you only want to test it to ensure that nothing is broken with your devices, that'll be just fine as well.

How would I actually try out the pull request?

If you want to test it from source code in your IDE you can do

git checkout -b kaikreuzer-nanoleaf main
git pull /~https://github.com/kaikreuzer/openhab-addons.git nanoleaf

Otherwise just use this jar for testing: org.openhab.binding.nanoleaf-3.1.0-SNAPSHOT.zip (rename it to .jar)

Are all chances backward compatible?

As mentioned in the description ("partially breaking"), unfortunately no.
For a textual configuration, it should be fairly easy to adapt, for auto-discovered ones it'll indeed be a bit nasty...

[kaikreuzer]

for auto-discovered ones it'll indeed be a bit nasty

I just checked - the ThingUID actually stays the same upon re-discovery, so I revert my comment that it might be nasty. Only the use of the changed channels might be required to be adapted, but overall the setup should still work.

@openhab/add-ons-maintainers I'd hence say it should be good to merge and I'll add a PR for distro wrt mentioning the breaking change.

[stefan-hoehn]

First of all, Kai, I am really sorry for delay. I currently have a lot of business and private obligations and as I take the development binding seriously I just wanted to do the review thoroughly. The reason I am a bit hesitant is the breaking change of the channels as you mentioned which I wanted to test first. I always tried not to break anything for the user, so two things about that:

1. Do you mind telling me why you changed the behaviour because I felt using a single touch and double touch pretty convenient to use?
2. We should at least add a migration section into the Readme file and an updated channel description to the Readme file.

I have reviewed all the non-breaking changes and I am sure given your expertise this is more than welcome for the binding. I unfortunately haven't had the chance to actually try out the binding. I intend to do so during the next week.

thanks for taking the your time on the nanoleaf binding changes,
Stefan

[kaikreuzer]

No worries, @stefan-hoehn, we are all very busy with other things in life besides openHAB ;-)

Wrt your questions:
Yes, in general I agree with not doing breaking changes. But in case where bindings violate the (unfortunately in bigger parts undocumented) rules of Thing modelling, I prefer to rectify them instead of keeping them as an example that will then be copied by others and which will thus proliferate.

1. The touch gestures are input events, they are not a stateful feature of the panels. Therefore, they must be of "trigger kind" and not of "state kind". You can consider them to be pretty much the same as a push button. The neat thing about trigger channels is that you can link them to an item, which you want to control and where you have a state channel linked as well (e.g. a light that you want to turn on/off by the touch gesture) - and you do not need any rule for it.
2. No, readmes should always reflect the latest documentation. Migration information must be put into the release notes. I will do so, once this PR is merged.

Hope this clarifies your questions a bit!

[stefan-hoehn]

Sorry for the long delay but as discussed offline I experienced a lot of issues during test not caused but some revealed by this PR, so I wanted to make sure that I know what is happening. For the curious here is a link to the related thread:

https://community.openhab.org/t/solved-httpclient-keeps-failing-get-request-after-onqueued-with-timeout/118320

[stefan-hoehn]

Thanks Kai for taking the time to improve the binding. Everything looks fine from my perspective.

[kaikreuzer]

@openhab/add-ons-maintainers Would be nice to get a review then, so that this can be finally merged!

[wborn]

I think it looks good! I have a few minor comments:

[wborn]

Thank you!

[stefan-hoehn]

Fine from my side