search

HASwitchPlate / openHASP-custom-component

Home Assistant custom component for openHASP
https://www.openhasp.com
MIT License
49 stars 9 forks source link

feat: allow full script syntax in event section #112

Closed akloeckner closed 2 weeks ago

akloeckner commented 10 months ago

This upgrades the current "list of services" restriction to allow the full syntax of HA scripts.

fixes #65

As a side-effect this also requires to generate a context for the script to run.

see #96

This can also be seen as a preparatory step to allow defining HASPObject-wide variables.

see #63

I believe, this should be even backwards-compatible, because a list of services is also a valid script.

dgomes commented 10 months ago

This is nice... but it moves from services to scripts

Don't think this is the best path... it will cause breaking changes in most setups

akloeckner commented 10 months ago

Isn't a "list of services" also a valid "script"? If it is, this should not break anything. Instead, it should make possible a whole range of new things, such as requested in the referenced issues.

But I might miss something not so obvious?

(As a side-effect, we would get in line with most of the other integrations. At least I am not aware of a single integration, that allows only a subset of the script syntax as actions. Again, I might miss something.)

dgomes commented 10 months ago

This custom integration was based on https://www.home-assistant.io/integrations/universal/ which supports services only.

How to deal with the breaking change ?

akloeckner commented 10 months ago

This custom integration was based on https://www.home-assistant.io/integrations/universal/ which supports services only.

Ah, I see. It even only admits a single service, which is not supported here, as far as I can see. (A list is required.)

How to deal with the breaking change ?

Not sure, if this question goes to me. If it does: I still don't see anything that could break. But I still might be mistaken. Just in case, a notice could be added in the release page as was done for 0.6.0. (If it does not: Never mind my answer. 😉)

dgomes commented 10 months ago

@nagyrobi since you have a big setup, can you test this PR :) ?

akloeckner commented 5 months ago

Hey, any news on this? I still don't see any breaking change here. Also, I have this running on my instance for half a year now, with no problems so far. The opposite: it allows me to do a whole range of things which wouldn't be possible with services alone. I'll give examples, if you like.

In my opinion: even if this were a breaking change, the opportunities outweigh the risks, by far. But that would be up to you too judge.

For curiosity, I also checked the history, and the actual breaking change was in this commit: https://github.com/HASwitchPlate/openHASP-custom-component/commit/e31fd76e8ab452ff72af89a29975c07267749cad

Bottom line: I really think this is a useful contribution, which I would rather like to see in the upstream codebase than in my personal fork, which noone uses but me. Especially, since 0.7.0 seems to be approaching (the documentation started to prompt my to switch to it's most current version 0.7.0 lately).

akloeckner commented 4 months ago

Checking back... Any news here?

nagyrobi commented 4 months ago

@nagyrobi since you have a big setup, can you test this PR :) ?

I'm sorry not anymore. I moved to ESPHome with all my displays meanwhile.

dgomes commented 4 months ago

!Offtopic

@nagyrobi would love to see a comparison feature wise :)

nagyrobi commented 4 months ago

@dgomes LVGL is still work in progress but I was able to migrate almost all functionality I previously had.

dgomes commented 4 months ago

like the fact that logic can be handled on the device, but coding everything in through yaml... not the best option IMHO. JSONL approach seams more sensible to me.

nagyrobi commented 4 months ago

I always looked for a way to ditch MQTT from the system, now this is finally possible. Also, for our needs I need zero templates, automations and customizations on HA side, I can do HA service calls directly from the device, no middleware required. HA seems to be faster without the ~200 templates I prevoiusly had to maintain for 6 displays.

I can have any degree of flexibility I want, from C++ lambdas even back to templates. Conceptually ESPHome is brilliantly thought out. Compilation is very flexible, it generates smaller binaries with only the desired functionality and no data loss. Encryption is at hands by default and simple to set up. Only one file to maintain per device.

After migration, family members got back the same design and functionality.

Free heap on the Lanbons previously was 25-30k, now it's aound 100k. This gives room for further expansion, not only additional pages, but also any modern sensor is rather easy to implement.

nagyrobi commented 4 months ago

Oh and moodlight color is restored across reboots every time regardless of HA or device state as it's being stored on the device itself.

akloeckner commented 4 months ago

🤔 Makes me wonder if I should abandon this PR altogether and switch as well. 😉

Jokes aside: Before I find the time for such an adventure, I would still highly appreciate the script feature to become part of this project. And be it only for those who follow...

dgomes commented 4 months ago

Oh and moodlight color is restored across reboots every time regardless of HA or device state as it's being stored on the device itself.

This could be easily added to the current firmware...

!Back_to_topic

the breaking change would be changing from single service to list of services... but if we are releasing 0.7 I guess we can accept some breaking changes @fvanroie

akloeckner commented 4 months ago

Just to remind: the breaking change from single service to list of services has been done way back in the past. From my I understanding, we're discussing a pure extension of functionality here, without breaking changes.

dgomes commented 4 months ago

I'm worried with the EVENT_SCHEMA validation change

akloeckner commented 4 months ago

Ah, ok. I'll check more thoroughly:

So, I'm confident that the current [SERVICE_SCHEMA] check is actually a special case of the SCRIPT_SCHEMA check.

Which makes sense, because a list of service calls is a perfectly valid script.

nagyrobi commented 4 months ago

This could be easily added to the current firmware...

Doesn't seem so, waiting for it for almost 3 years now: https://github.com/HASwitchPlate/openHASP/issues/99#issuecomment-851626353

fvanroie commented 4 months ago

This could be easily added to the current firmware...

Doesn't seem so, waiting for it for almost 3 years now: HASwitchPlate/openHASP#99 (comment)

That issue is closed now... it doesn't seem anyone missed that feature in the past 3 years.

nagyrobi commented 4 months ago

That issue is closed now... it doesn't seem anyone missed that feature in the past 3 years.

I closed it 2 weeks ago, after I finally decided that I switch to ESPHome.

fvanroie commented 1 month ago

Is this still active?

akloeckner commented 1 month ago

Yes. It's running on my system since I started the PR (more or less). Without any problems.

akloeckner commented 1 month ago

I rebased onto main to test #122. No further changes.

akloeckner commented 2 weeks ago

Is this still active?

I was wondering: is there anything else you need from me to proceed on this?

fvanroie commented 2 weeks ago

I have no knowledge of the HA inner workings to decide one way or the other. I will let you guys figure it out. I was just curious since this issue has been open for some time now.

akloeckner commented 2 weeks ago

Ok, thanks for clarifying! To my understanding, Diogo had approved the changes on March 3. So, I assumed this had handed it over to you somehow.

@dgomes, anything missing from me? Please let me know!

dgomes commented 2 weeks ago

I think this is good to go