-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: allow full script syntax in event section #112
Conversation
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 |
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.) |
This custom integration was based on https://www.home-assistant.io/integrations/universal/ which supports services only. How to deal with the breaking change ? |
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.)
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. 😉) |
@nagyrobi since you have a big setup, can you test this PR :) ? |
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: e31fd76 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). |
Checking back... Any news here? |
I'm sorry not anymore. I moved to ESPHome with all my displays meanwhile. |
!Offtopic @nagyrobi would love to see a comparison feature wise :) |
@dgomes LVGL is still work in progress but I was able to migrate almost all functionality I previously had. |
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. |
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. 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. |
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. |
🤔 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... |
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 |
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. |
I'm worried with the EVENT_SCHEMA validation change |
Ah, ok. I'll check more thoroughly:
So, I'm confident that the current Which makes sense, because a list of service calls is a perfectly valid script. |
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. |
I closed it 2 weeks ago, after I finally decided that I switch to ESPHome. |
Is this still active? |
Yes. It's running on my system since I started the PR (more or less). Without any problems. |
This upgrades the current "list of services" restriction to allow the full syntax of HA scripts. fixes HASwitchPlate#65 As a side-effect this also requires to generate a context for the script to run. see HASwitchPlate#96 This can also be seen as a preparatory step to allow defining HASPObject-wide variables. see HASwitchPlate#63
I just read the contributing guidelines...
I rebased onto main to test #122. No further changes. |
I was wondering: is there anything else you need from me to proceed on this? |
I have no knowledge of the HA inner workings to decide one way or the other. I will let you guys figure it out. |
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! |
I think this is good to go |
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.