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

Allow to use HA Scripts as Button Entity #20

Merged
merged 1 commit into from
Sep 11, 2023

Conversation

splattner
Copy link
Contributor

No description provided.

Copy link
Contributor

@zehnm zehnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! The script support is working fine 👍
Please fix the cosmetic formatting issues reported by cargo fmt, then I will merge this PR.
You can ignore the reported issue by Clippy, I've fixed that on the main branch.

The only thing we could discuss is to use a switch instead of a button for HA scripts. This would allow to:

  • stop a running script (at least according to the HA docs. I don't know if the script will be killed, or if it needs specific support to be stoppable).
  • see when a (long running) script is active on the remote.

For me personally it doesn't really matter right now. I still have to migrate everything to HA and I'm not using scripts yet (just a matter of time...).

src/client/service/button.rs Outdated Show resolved Hide resolved
src/client/service/mod.rs Outdated Show resolved Hide resolved
@splattner
Copy link
Contributor Author

Thank you for your contribution! The script support is working fine 👍 Please fix the cosmetic formatting issues reported by cargo fmt, then I will merge this PR. You can ignore the reported issue by Clippy, I've fixed that on the main branch.

The only thing we could discuss is to use a switch instead of a button for HA scripts. This would allow to:

  • stop a running script (at least according to the HA docs. I don't know if the script will be killed, or if it needs specific support to be stoppable).
  • see when a (long running) script is active on the remote.

For me personally it doesn't really matter right now. I still have to migrate everything to HA and I'm not using scripts yet (just a matter of time...).

thanks for reviewing it. Fmt errors are fixed, still new to Rust, that was my first code in rust ever ;)

I actually never had the need to stop a running scripts, mine are quite short. So for me, a button is enough. Yes, HA allows to stop a running script, there is a turn_on & turn_off and also a toggle service. And what I did here, is just calling it by name, which is what I did so far in HA.
My personal opinion, I think its confusing to have this as a Switch, therefore I'd rather leave it as a Button. with only the the call by name.

Copy link
Contributor

@zehnm zehnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again! I'll merge it, button entitiy it is 👍

You haven't picked the easiest Rust project to start with ;-)
Also some of my "earlier" work and a bit complicated with the Actor stuff.

@zehnm zehnm merged commit 5c7d891 into unfoldedcircle:main Sep 11, 2023
5 checks passed
@uvjim
Copy link

uvjim commented Sep 19, 2023

A quick question on this one now that it has been released... How do I pass parameters to the script that needs to be called? I can't seem to find that in the Web Configurator.

@zehnm
Copy link
Contributor

zehnm commented Sep 19, 2023

How do I pass parameters to the script that needs to be called?

That is currently not possible.
For now, HA scripts can only be triggered. Supporting parameters will require more work, most likely with a new entity type, which supports dynamic parameters.

Since this PR is merged, let's continue in #20

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.

None yet

3 participants