Feature/rgb lights #69

xiki808 · 2021-03-06T22:36:07Z

Say: change boxroom light to blue - to trigger the color change.

The 'to' is key, that separates the entity from the color, and so it allows for example: change boxroom light TO light blue.

Tony763 · 2021-03-06T22:41:54Z

Nice, I will send you a PR with Czech translation tomorrow.

Tony763 · 2021-03-08T19:55:35Z

@xiki808 PR sended.

cs-cz localization

stratus-ss · 2021-04-13T16:28:46Z

I'll take a look at this shortly.

We are trying to encourage people to put more effort into the descriptions of the pull requests. We made a video about it here

stratus-ss · 2021-04-17T21:45:07Z

This one passes all the tests which is expected.

The all function also works as expected and individual lights also work as expected

However, I dont know if this is a problem with this PR or the skill itself, but when I call an entity that doesn't exist it seems to grab a random light... for example:

change bathroom light to orange                                 --- 0.21
 >> LaundryRoom changed to orange.  
change outside light to green                             
 >> Steve's Bed Light is  unavailable.

Do you have the same results @xiki808 ?

stratus-ss · 2021-04-17T21:58:39Z

Let's get some tests added for the new functionality. You can find the existing tests under homeassistant.mycroftai/test/intent

Let us know if you need help with that!

Tony763 · 2021-04-18T06:51:01Z

@xiki808 If you need help with tests, ping me.

xiki808 · 2021-04-18T10:02:15Z

Thanks guys, will have a look into this early this week.

xiki808 · 2021-04-21T20:19:11Z

@stratus-ss For me it is working fine, I tried the following:

change test light to blue
>> Changed Test to blue.
change desk light to red
>> Sorry, I can't find the Home Assistant entity "desk light".
change test light to red
>> Changed Test to red.

turn off test
>> turned Test off.
change test to red
>> Test changed to red.
turn off test
>> turned off Test.
change desk to blue
>> Excuse me, I wasn't able to find "desk".

I will try to find some time to try with the same naming.

xiki808 · 2021-04-21T20:41:09Z

@Tony763 would love it if you can handle the test, I cannot see how to run and check my test intent.

Tony763 · 2021-08-18T06:24:24Z

Hi @xiki808 , Kris just merged two of my commits so there is a conflict in init.py now, but I can't see it. Please, check it and try to fix or send me a screenshot. Maybe a fetching upstream could be sufficient. If anything, I will gladly help.

xiki808 · 2021-08-18T17:06:47Z

I don't have the setup currently, been using the Pi for other projects. I will try to reinstall Mycroft next week and check it!

Tony763 · 2021-09-23T08:10:12Z

Hi @xiki808 were you able to reinstall Mycroft? If not, could I help you?

xiki808 · 2021-09-27T06:53:57Z

Hi @Tony763, I haven't had the time yet, but will find some time this week

into MycroftAI-20.08

Tony763 · 2021-09-27T19:05:21Z

Hi @krisgesling, merge issue solved and tested. Seems Ok to me.

krisgesling

Hey, a great new feature, thanks!

A few suggestions below. I'm also wondering if deleting the .gitignore file was intentional?

krisgesling · 2021-09-28T06:53:24Z

__init__.py

+        if "all" in entity_parts:
+            voc_match = "all lights"
+
+        if self.voc_match(voc_match, "all_lights"):
+            ha_data = {'entity_id': 'all'}
+            ha_entity = {'dev_name': 'all lights'}


So presently if you said:

Change all kitchen lights to green

That would see "all" and set the entity to "all lights" which presumably be all lights in the house?

Yeah, doesn't make sense, I removed the 'all' catch phrase

krisgesling · 2021-09-28T06:56:37Z

__init__.py

+            ha_data = {'entity_id': ha_entity['id']}
+
+        color = message.data['color']
+        color_parts = list(color.split())


Does color_parts get used at all?
If not the previous line can probably be removed also.

krisgesling · 2021-09-28T06:59:38Z

__init__.py

+            if not ha_entity or not self._check_availability(ha_entity):
+                return
+
+            ha_data = {'entity_id': ha_entity['id']}


I'd love if we could find a more descriptive name for this ha_data object.

action_data?

Tony763 · 2021-09-28T18:56:40Z

Hi @xiki808 and @krisgesling I tested it again with latest commit and it's working.

I have only one thing that could be improved. When I ask Mycroft to change color of light that does not support colors, it still say that color was changed.

Found entity_id contain attributes and for lights there is supported_color_mode.

skill-homeassistant/ha_client.py

Line 66 in 715bc81

if state['entity_id'].split(".")[0] in types:

Without support:

With support:

As defined in HA light, color can be changed only when at least one of hs, xy, rgb, rgbw, rgbww modes is present and if not, Mycroft could inform uses that color change is not supported.

https://github.com/home-assistant/core/blob/bc59387437cea91d675584bb9057338aa5992817/homeassistant/components/light/__init__.py#L53-L64

I would suggest to pass whole attributes dictionary to __init__.py as same thing will more-less be happening in future with others intents.

What do You think?

stratus-ss · 2021-09-28T19:02:00Z

How does this handle rgbww and/or rgbw?

Tony763 · 2021-09-28T19:26:25Z

Hi @stratus-ss , long not hear from You :). Color is directly passed to HA, so handling is done in side HA not in skill.

This way with rgbw and rgbww only rgb part is controlled. White and temperature of white is omitted. If its on, it stay on or off. Brightness of color can be controlled by set intensity intent.

Although I just realized that this will perfectly work in English. But in other languages, HA will not understood to color names!

stratus-ss · 2021-09-28T20:04:20Z

Let me see if I can carve out some time this week to test this out now that you folks have been doing some good cleanup work

Extarys · 2021-09-30T21:32:12Z

For the color issue, is there a way to make a dummy mycroft skill so users can translate the colors name on Mycroft Translate and use those inside other skills?

krisgesling · 2021-10-01T01:56:40Z

Hey @Extarys - that's not a bad idea

You could make a Color Skill that displays the color along with the RGB and/or hex values. Submit this to the Marketplace and it will get added to translate.mycroft.ai, you can then poach the translated colors back out of it.

Tony763 · 2021-10-01T07:24:17Z

Or maybe we could try auto translate. I have seen it in some other skill. One word translation should not be big challenge.

stratus-ss · 2021-10-01T09:50:52Z

The CSS3 color spec has 140(ish) colors that might be hard to translate one at a time

Extarys · 2021-10-04T19:59:26Z

@stratus-ss I wouldn't mind translating them in my own language, it would not take that much time and the CSS color name has never changed much. so it would be once per language and would last 25+ year 😂 Of course it wouldn't be done in 5 minutes so it still require some efforts.

Ppl can probably use LibreTranslate or Google Translate too as it should be simple for those automated tools - it's not a complicated sentence or something.

Very Mycroft noob so not sure how the auto translate thing would work, but I have no idea how to make a plugin so I would probably leave that to a user who wont take a month to make it 😄 but I won't mind translating it in french and then other languages with automated tools.

Tony763 · 2021-10-04T20:22:55Z

I have seen auto translate function in one of skills I translated last year. It was quite easy to use. I will try to find it and we can try it and compare results.

stratus-ss · 2021-10-15T01:41:04Z

Gez and I were discussing this one and we were wondering how non-english speakers would handle most of the colors. For example "light golden rod" or similar words would not translate automatically and would require someone versed in colours to be able to translate them appropriately.

Speaking for myself I dont know what colours most of these are (just reading the names) and I am a native speaker. So the question is, would users learn their native words for these bizarre colours, or would they just learn the english name and call it a day?

krisgesling · 2021-10-15T02:31:15Z

Also wanted to flag that we've started a Color Picker Skill for an upcoming video series - so we can get an early MVP version of that into the Marketplace and start getting colors translated if it would be helpful.

Currently the colors are included as their exact CSS3 forms eg "lightgoldenrodyellow" rather than "light golden rod yellow" but thinking through the best way to approach this and hence the question above.

If you're a German speaker and want to change your lights to a "lightgoldenrodyellow" color (#fafad2), what would you say?

krisgesling · 2021-10-29T01:49:40Z

I've updated the color names to have spaces - much easier to remove spaces programmatically.

It's also nearing an MVP so should be able to push it to the Marketplace soonish. It's here if you want to check it out:
https://github.com/krisgesling/color-picker-skill

Extarys · 2021-10-30T00:09:02Z

Nice! I don't see it yet on https://translate.mycroft.ai/ but I'll help translating to french as soon as I see it there ❤️

pfefferle · 2021-11-19T08:43:49Z

Is there something I can help to get this merged?

krisgesling · 2021-11-29T04:58:19Z

@pfefferle if you can verify the functionality works as expected that would be great. It didn't seem to be working for @stratus-ss but there might have been updates since that comment

It would be great to add a check to see if the given device supports color as suggested by Tony here:
#69 (comment)

But I'd be comfortable with that being a follow up PR.

arborealfirecat · 2023-04-20T08:59:03Z

Hey all, sorry to necro an old PR, but are you still open to merging this functionality? If so I'm happy to pick it up and finish off the last bits of the PR?

stratus-ss · 2023-04-20T12:32:43Z

Hey all, sorry to necro an old PR, but are you still open to merging this functionality? If so I'm happy to pick it up and finish off the last bits of the PR?

@krisgesling and I are still floating around. Speaking for myself, I don't mind continuing to approve PRs

stratus-ss · 2023-05-01T13:04:29Z

I re-looked at this. There is a lot of whitespace noise in this PR.

overall I don't have any major concerns. If there is still interest and the original author isn't involved we can get a new PR opened

xiki808 added 3 commits March 6, 2021 21:56

Add change light color intent

03adeaf

Git ignore .vscode

db3de74

Add new lines

f260e50

xiki808 and others added 4 commits March 7, 2021 08:53

Remove unused code - color one word not required

a465f2b

Validate if device is given

6717585

Add new line

2c75b29

cs-cz localization

4b82878

xiki808 and others added 3 commits March 8, 2021 14:17

Merge pull request #1 from Tony763/feature/rgb-lights

7993872

cs-cz localization

Delete launch.json

30ecd6a

Delete .gitignore

218727d

Tony763 mentioned this pull request Sep 20, 2021

Allow Changing Colour of Lights #75

Open

xiki808 added 2 commits September 27, 2021 20:12

Merge branch '20.08' of https://github.com/MycroftAI/skill-homeassistant

b433213

into MycroftAI-20.08

Merge branch 'MycroftAI-20.08' into feature/rgb-lights

01e9fbe

krisgesling reviewed Sep 28, 2021

View reviewed changes

xiki808 added 2 commits September 28, 2021 11:32

PR updates: cleanup and refactoring, add back gitignore

f383999

Remove 'all' catch phrase

12b562e

xiki808 added 2 commits September 28, 2021 11:43

remove history files

a86d5c6

rename data var

715bc81

Feature/rgb lights #69

Are you sure you want to change the base?

Feature/rgb lights #69

Conversation

xiki808 commented Mar 6, 2021 • edited Loading

Tony763 commented Mar 6, 2021

Tony763 commented Mar 8, 2021

stratus-ss commented Apr 13, 2021

stratus-ss commented Apr 17, 2021

stratus-ss commented Apr 17, 2021

Tony763 commented Apr 18, 2021

xiki808 commented Apr 18, 2021

xiki808 commented Apr 21, 2021 • edited Loading

xiki808 commented Apr 21, 2021

Tony763 commented Aug 18, 2021

xiki808 commented Aug 18, 2021

Tony763 commented Sep 23, 2021

xiki808 commented Sep 27, 2021

Tony763 commented Sep 27, 2021

krisgesling left a comment

Choose a reason for hiding this comment

krisgesling Sep 28, 2021

Choose a reason for hiding this comment

xiki808 Sep 28, 2021

Choose a reason for hiding this comment

krisgesling Sep 28, 2021

Choose a reason for hiding this comment

xiki808 Sep 28, 2021

Choose a reason for hiding this comment

krisgesling Sep 28, 2021

Choose a reason for hiding this comment

xiki808 Sep 28, 2021

Choose a reason for hiding this comment

Tony763 commented Sep 28, 2021 • edited Loading

stratus-ss commented Sep 28, 2021

Tony763 commented Sep 28, 2021

stratus-ss commented Sep 28, 2021

Extarys commented Sep 30, 2021

krisgesling commented Oct 1, 2021

Tony763 commented Oct 1, 2021

stratus-ss commented Oct 1, 2021

Extarys commented Oct 4, 2021 • edited Loading

Tony763 commented Oct 4, 2021

stratus-ss commented Oct 15, 2021 • edited Loading

krisgesling commented Oct 15, 2021

krisgesling commented Oct 29, 2021

Extarys commented Oct 30, 2021

pfefferle commented Nov 19, 2021

krisgesling commented Nov 29, 2021

arborealfirecat commented Apr 20, 2023

stratus-ss commented Apr 20, 2023

stratus-ss commented May 1, 2023

xiki808 commented Mar 6, 2021 •

edited

Loading

xiki808 commented Apr 21, 2021 •

edited

Loading

Tony763 commented Sep 28, 2021 •

edited

Loading

Extarys commented Oct 4, 2021 •

edited

Loading

stratus-ss commented Oct 15, 2021 •

edited

Loading