core
core copied to clipboard
Alexa Intent: Use the 'id' field and expose nearest resolutions as variables
Expose nearest Resolution (ID and Value) as Variables
Breaking change
Proposed change
Using the id value can be valuable if you don't want to use the id for entities but to process it in scripts for example. The ID Value is exposed as SlotName_ID variable. In addition to the single value it could be useful to have the nearest resolution always available.
For example:
"TramStation": {
"name": "TramStation",
"value": "hauptbahnhof ost",
"resolutions": {
"resolutionsPerAuthority": [
{
"authority": "amzn1.er-authority.echo-sdk.amzn1.ask.skill.91b2d64a-aaa7-4390-bb66-a196e950d8ea.TramStation",
"status": {
"code": "ER_SUCCESS_MATCH"
},
"values": [
{
"value": {
"name": "Hauptbahnhof Ost",
"id": "80029079"
}
},
{
"value": {
"name": "Hauptbahnhof West",
"id": "80029080"
}
}
]
}
]
},
"confirmationStatus": "NONE",
"source": "USER"
}
In that case I wouldn't get the correct resolution, as it's more than one. So it defaults to the spoken value. However the first resolution is always the one that fits best. For that reason I exposed two more variables named "SlotName_NEAREST" and "SlotName_NEAREST_ID". That way people can use it when they need it and can't rely on the default spoken value.
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [x] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
- This PR fixes or closes issue: fixes #86629
- This PR is related to issue: #86629
- Link to documentation pull request:
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] The code has been formatted using Black (
black --fast homeassistant tests
) - [x] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [ ] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [ ] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest
. - [ ] New or updated dependencies have been added to
requirements_all.txt
.
Updated by runningpython3 -m script.gen_requirements_all
. - [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [ ] Untested files have been added to
.coveragerc
.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
Hi AzonInc
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
Hey there @home-assistant/cloud, @ochlocracy, @jbouwh, mind taking a look at this pull request as it has been labeled with an integration (alexa
) you are listed as a code owner for? Thanks!
Code owner commands
Code owners of alexa
can trigger bot actions by commenting:
-
@home-assistant close
Closes the issue. -
@home-assistant rename Awesome new title
Change the title of the issue. -
@home-assistant reopen
Reopen the issue. -
@home-assistant unassign alexa
Removes the current integration label and assignees on the issue, add the integration domain after the command.
Please help me to understand why how this ID field would help. This is a part that I am not yet quite familiar with. It would help if you could explain what steps I need to make use of this.
Please fix the errors, and tests. The PR code contains and breaks the alexa integration.
Sorry for that, I'm quite new and I'm not sure where these errors come from. But you can see that I didn't change any dependencies - just a small change. Unfortunately I have no Idea how to fix that.
I rebased to get up with the latest commits - dunno if it's fixed now due to some work in progress in dev branch before. Do you have an Idea what is causing that? Looks like this might be somewhat related.
Please help me to understand why how this ID field would help. This is a part that I am not yet quite familiar with. It would help if you could explain what steps I need to make use of this.
The ID field would help whenever we want to pass some custom IDs (for example train Station Location IDs) to Homeassistant Automations (or Node-RED Addon Flows). It was initially removed because people thought there is no use because they can generate their entity ids by the name. However there are people out there, that dont want to use the Slot Value ID as entity ID, but as a real ID for TV Apps or Train Station IDs. That way it's not neccesary anymore to get the IDs somewhere else by Name if we could just serve them with Alexa. It's easier to work with the IDs rather than collecting them later or comparing strings that could have special characters.
I hope I could explain it.
From what I see in the local tests they need to be fixed:
FAILED tests/components/alexa/test_intent.py::test_intent_request_with_slots - AssertionError: assert 500 == <HTTPStatus.OK: 200>
FAILED tests/components/alexa/test_intent.py::test_intent_request_with_slots_and_synonym_resolution - AssertionError: assert 500 == <HTTPStatus.OK: 200>
FAILED tests/components/alexa/test_intent.py::test_intent_request_with_slots_and_multi_synonym_resolution - AssertionError: assert 500 == <HTTPStatus.OK: 200>
FAILED tests/components/alexa/test_intent.py::test_intent_request_calling_service - AssertionError: assert 500 == <HTTPStatus.OK: 200>
FAILED tests/components/alexa/test_intent.py::test_intent_from_built_in_intent_library - AssertionError: assert 500 == <HTTPStatus.OK: 200>
Results (11.00s):
182 passed
5 failed
- tests/components/alexa/test_intent.py:216 test_intent_request_with_slots
- tests/components/alexa/test_intent.py:250 test_intent_request_with_slots_and_synonym_resolution
- tests/components/alexa/test_intent.py:353 test_intent_request_with_slots_and_multi_synonym_resolution
- tests/components/alexa/test_intent.py:481 test_intent_request_calling_service
- tests/components/alexa/test_intent.py:553 test_intent_from_built_in_intent_library
The changed code seems not to be called during normal operation. So I do not really get when this code is used. Could you tell me step by step how to set up a test environment to be able to test/debug your code in real live.
I finally figured how to properly run the Tests and Test Enviroment. All Tests are passing now. I added two more Tests to cover all scenarios of the new variables.
I updated the branch to reflect the most recent changes to the tests file.
Can you answer my questions on how this is supposed to be used and especially how I can test your code, and decide if this would fit or not. This might save you a lot of time,
Scenareo: Ask Alexa for the next Tram to "Hauptbahnhof West" Ask Alexa for the next Tram to "Hauptbahnhof Ost" Ask Alexa for the next Tram to "Hauptbahnhof" (Synonym for West) Ask Alexa for the next Tram to "Bahnhof" (Synonym for West)
Problem:
- Currently multiple resolutions are not supported, even when the first resolution is always the correct one, the spoken value is used instead. As there are always multiple resolutions for everything that contains "Hauptbahnhof" it's failing to resolve "Hauptbahnhof Ost" and "Hauptbahnhof West" even when there is no synonym set. Because Amazon is sending the two but the correct resolution on index 0.
- Since the ID Support of the Alexa Integration got removed in the past, it's not possible to use the ID field of the Slot at all which is a pity.
Solution: I kept that behaviour with the Code changes and added three new variables. So there is no breaking change. One new Variable for the Slot ID: "SlotName"_ID Two new Variables for the nearest resolution value and id: "SlotName"_NEAREST, "SlotName"_NEAREST_ID
Alexa Intent Setup:
Alexa Intent Slot Setup:
Homeassistant Intent:
TramIntentTest:
action:
- service: input_text.set_value
data:
value: "{{ TramStation_NEAREST_ID }}"
target:
entity_id: input_text.tram_destination
- service: button.press
data: {}
target:
entity_id: button.request_tram
speech:
type: plain
text: Wait a Second, I'm looking for a tram to {{ TramStation_NEAREST }}
Entities: Create one Input Button in HA with id: input_text.tram_destination Create a Button in NodeRED (Companion Integration) with id: button.request_tram
Now you can ask Alexa for the next Tram to some Station. The Intent is setting the input value to the nerest resolution tram station id and press the button in nodered to start the flow.
I tried to setup a custom skill: using https://www.home-assistant.io/integrations/alexa.intent/#create-your-amazon-alexa-custom-skill (not successfully succeeded unfortunately ). Could that help to be able to use the intents as you demonstrate? Currently I only have a haaska skill set up for the common functionality. @balloob May be you could have a look at this proposal?
I tried to setup a custom skill: using https://www.home-assistant.io/integrations/alexa.intent/#create-your-amazon-alexa-custom-skill (not successfully succeeded unfortunately ). Could that help to be able to use the intents as you demonstrate? Currently I only have a haaska skill set up for the common functionality. @balloob May be you could have a look at this proposal?
Yes. I use the HA Alexa Custom Skill for the TramStation Intent. However I don't think it's possible with HAASKA.
I have successfully set up a test environment, but I have to pick this up later.
I'm happy to hear. Just take your time and test whenever you're free :)
Got an Hello world intent running, will try some more later to see what the ID field does.
I uploaded My Homeassistant Skill JSON including the TramStationTest Intent: https://gist.github.com/AzonInc/c297f5642695bf4af0dfad4631b0f5a6
You can upload the model using the json editor:
It will overwrite any existing intent configuration tho. It's invocation name is home assistant - feel free to change it or just play around with your own test intent :)
Hi @jbouwh did you have time already to test the new intent slot variables?
Hi @jbouwh did you have time already to test the new intent slot variables?
I have a working intent, but did not test with the code yet, may be I will next week?
My intent works, but to trigger resolve_slot_data
I need to have another HA intent with some shared intents.
@jbouwh you can take the one of my Interaction model above as an example
Okay, I think that I known now how this should work. I did not use node red, but created a light where the colors were the slot values, and that worked. I think the change is okay as it adds some additional variables for template rendering. Could you make a documentation PR and rebase this PR?
Set the PR to ready for review when you are ready.
@AzonInc do you need help?
@AzonInc do you need help?
Hey, sorry for the delay - I have some trouble at the moment but I will look into it ASAP.
@AzonInc do you need help?
Hey, I'm back. How can I make a documentation contribution? @jbouwh Where's the best place in the Documentation? I guess you mean this one: https://github.com/home-assistant/home-assistant.io/blob/current/source/_integrations/alexa.intent.markdown
@AzonInc do you need help?
Hey, I'm back. How can I make a documentation contribution? @jbouwh Where's the best place in the Documentation? I guess you mean this one: https://github.com/home-assistant/home-assistant.io/blob/current/source/_integrations/alexa.intent.markdown
Correct, target against the next
branch
I created a docs PR: https://github.com/home-assistant/home-assistant.io/pull/27326 About that one missing type hint - If you have time and want to find out the type I'd be happy about that.
Actually it could be way easier with only that one new Slot ID variable and a behaviour change which would lead to a breaking change tho.
I don't know why it is useful to prevent using the Slot resolutions when there is more than one possible resolution. But maybe you can explain me the reasons behind the decision.
Let's say we have an Intent Configuration like this: Utterance: Show me a route to {{ TrainStation}} Slot Name: TrainStation Slot Values: South Station, West Station, Apple Street and Junior Street
Behavior of the current implementation:
- When there is no or more than one resolution, fall back to spoken Text
If I say "Alexa, show me a route to (West Street | West Station | Apple Street | Junior Street)", it will never use any of the resolutions bacause amazon is resolving them all as there are two values ending with street and two starting with west. So with the current implementation it will always fall back to the spoken Text in my example.
Suggestion: New implementation
- The nearest resolution will always be used, even when there is more than one.
- Only fall back to the spoken Text if there is no resolution available.
As far as I have seen and tested, the first resolution has always been the correct one. The new implementation however will be a breaking change for some people using the Slots as the resolved values might be a little bit different sometimes e.g. lowercase/uppercase/whitespace.
Let me know what you think about it.
Actually it could be way easier with only that one new Slot ID variable and a behaviour change which would lead to a breaking change tho.
I don't know why it is useful to prevent using the Slot resolutions when there is more than one possible resolution. But maybe you can explain me the reasons behind the decision.
Let's say we have an Intent Configuration like this: Utterance: Show me a route to {{ TrainStation}} Slot Name: TrainStation Slot Values: South Station, West Station, Apple Street and Junior Street
Behavior of the current implementation:
* When there is no or more than one resolution, fall back to spoken Text
If I say "Alexa, show me a route to (West Street | West Station | Apple Street | Junior Street)", it will never use any of the resolutions bacause amazon is resolving them all as there are two values ending with street and two starting with west. So with the current implementation it will always fall back to the spoken Text in my example.
Suggestion: New implementation
* The nearest resolution will always be used, even when there is more than one. * Only fall back to the spoken Text if there is no resolution available.
As far as I have seen and tested, the first resolution has always been the correct one. The new implementation however will be a breaking change for some people using the Slots as the resolved values might be a little bit different sometimes e.g. lowercase/uppercase/whitespace.
Let me know what you think about it.
I have invest some time to understand what you are suggestion and what would break. If we break anything it would be nice if we could have some backwards compatibility and a deprecation warning (repair). Don't know if that is possible though. If we can explain well enough what cases will break we could also explain this in the breaking changes release notes.
btw: i see that the merging of dev pushed a lot of non related commits. to solve this you can do:
-
git fetch upstream
-
git rebase -i upstream/dev
(select the commits you want to keep, or omit the-i
flag) -
git push --force
So do not pull in dev in your branch.