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

Add some options to control "chattiness" of the plugin #40

Merged
merged 12 commits into from
May 18, 2024

Conversation

jumper047
Copy link
Contributor

@jumper047 jumper047 commented Mar 10, 2024

I added two options to your plugin:

  • Skip acknowledge dialogue completely (I believe it is have not too much of useful information, because just after it another dialog started which actually duplicates information from first one. In fact, I'd suggest to get rid of it completely). Option disabled by default.
  • Skip confirmation dialogue in actions for certain entities. Usecase: control lights in the same room as Mycroft. I don't need additional confirmation that lights are turned on if they are turned on.

Settings file example with new params:

{
    "verbose": false,
    "silent_entities": [
        "hallway lamp"
    ],
    "__mycroft_skill_firstrun": false,
    "connected": true
}

Fixes #34

Copy link
Owner

@mikejgray mikejgray left a comment

Choose a reason for hiding this comment

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

Hi, thank you for this contribution! I appreciate it very much.

I'm not opposed to anything here, but would you please add a log message if verbose is True, as there will be no indication to the user if their request failed or succeeded without that. Since this is an async process that hits the PHAL plugin, which may or may not ever respond, that log would make troubleshooting issues much easier. self.log.info() level should be fine.

@jumper047 jumper047 force-pushed the fetaure/silent-servant branch from 6be2c7f to 8f0a4ee Compare March 12, 2024 21:37
@jumper047
Copy link
Contributor Author

Makes sense, thank you for feedback! Now it will be phrase or log entry.
BTW, I changed and force-pushed commits, is it ok or is it better to add new commits with fixes?

@mikejgray
Copy link
Owner

Makes sense, thank you for feedback! Now it will be phrase or log entry. BTW, I changed and force-pushed commits, is it ok or is it better to add new commits with fixes?

Generally, I prefer new commits, as it makes it more clear what the history of the PR was. We squash commits on merge so there's no need to force-push anything

@mikejgray mikejgray merged commit 5001603 into mikejgray:main May 18, 2024
18 checks passed
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.

[FEAT] Make skill less chatty
2 participants