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

Migrate errors #1906

Merged
merged 2 commits into from
Feb 1, 2024
Merged

Migrate errors #1906

merged 2 commits into from
Feb 1, 2024

Conversation

synesthesiam
Copy link
Contributor

@synesthesiam synesthesiam commented Feb 1, 2024

We plan to add some new error messages, and migrate previous errors to be more precise. The following core PR in HA implements some of the new errors: home-assistant/core#109269

The existing no_domain and no_device_class errors have been renamed to no_domain_in_area and no_device_class_in_area. Additionally, no_entity has been renamed to no_device.

A complete list of the proposed errors are here:

  • no_intent - Intent was not recognized.
  • handle_error - Unexpected error while handling intent.
  • no_area - Area does not exist.
  • no_domain - No devices exist for a domain.
  • no_domain_exposed - No devices are exposed for a domain.
  • no_domain_in_area - No devices in an area exist for a domain.
  • no_domain_in_area_exposed - No devices in an area are exposed for a domain.
  • no_device_class - No devices of a class exist.
  • no_device_class_exposed - No devices of a class are exposed.
  • no_device_class_in_area - No devices of a class exist in an area.
  • no_device_class_in_area_exposed - No devices of a class are exposed in an area.
  • no_entity - Entity does not exist.
  • no_entity_exposed - Entity is not exposed.
  • no_entity_in_area - Entity does not exist in area.
  • no_entity_in_area_exposed - Entity in area is not exposed.
  • duplicate_entities - More than one entity matched with the same name.
  • duplicate_entities_in_area - More than one entity in an area matched with the same name.

@synesthesiam
Copy link
Contributor Author

@tetele From the user's perspective, I don't think the device/entity distinction is important. This is only for error responses, and doesn't mean that the intents themselves will not receive entity names or anything.

@v1k70rk4 Go ahead and merge. I will wait as long as I can and clean up on my end.

@jlpouffier It would be no_device_in_area in this case. A confusing case is when you have both a device and and area that don't exist: "turn on the monkey in the spaceship". In this case, I went with no_area.

@tetele
Copy link
Contributor

tetele commented Feb 1, 2024

@synesthesiam it's fine for the sentences, those can be perfected later on. My issue was with the keys, i feel as though they create confusion between devices and entities

@jlpouffier
Copy link
Contributor

@synesthesiam Great, with @piitaya we created another PR, we will merge later (#1909), after yours

@nagyrobi nagyrobi marked this pull request as ready for review February 1, 2024 17:16
@home-assistant home-assistant bot requested a review from tetele February 1, 2024 17:16
@jlpouffier jlpouffier self-requested a review February 1, 2024 17:28
@synesthesiam
Copy link
Contributor Author

@synesthesiam it's fine for the sentences, those can be perfected later on. My issue was with the keys, i feel as though they create confusion between devices and entities

Do you see any benefit to having both no_device and no_entity keys? If it was combined into no_device_or_entity, then there would be confusion about which variable to use: {{ device }} or {{ entity }}.

@tetele
Copy link
Contributor

tetele commented Feb 1, 2024

I have referenced that PR in which i have made devices available (actual HA devices, not entities) as a prefilled slot. I still want to do that, although i will have to rewritw the PR.

As such, we will need both no_entity and no_device errors going forward. I think it's a bad idea to use no_device as a key for something that is a missing entity, not a missing device. The lingo exposed to the user through the error message is less relevant in this argument.

v1k70rk4 added a commit to v1k70rk4/intents that referenced this pull request Feb 1, 2024
@synesthesiam
Copy link
Contributor Author

OK, I am changing "device" to "entity" in the relevant keys. We'll add device keys later as needed.

@synesthesiam synesthesiam merged commit c2fa7c8 into main Feb 1, 2024
2 checks passed
@synesthesiam synesthesiam deleted the synesthesiam-20240131-errors branch February 1, 2024 19:20
@tetele tetele mentioned this pull request Feb 1, 2024
@tetele
Copy link
Contributor

tetele commented Feb 1, 2024

@synesthesiam I've edited the PR description to reflect the rollback to entity

@mib1185 mib1185 mentioned this pull request Feb 2, 2024
davefx added a commit to davefx/intents that referenced this pull request Feb 5, 2024
HLandstrom added a commit to HLandstrom/intents that referenced this pull request Feb 5, 2024
larsdunemark pushed a commit that referenced this pull request Feb 5, 2024
…issing colors (#1938)

* Translating new error messages from #1906 to Swedish
schizza pushed a commit to schizza/intents that referenced this pull request Mar 16, 2024
* Migrate errors

* Rename device to entity
schizza pushed a commit to schizza/intents that referenced this pull request Mar 16, 2024
…sh and added missing colors (home-assistant#1938)

* Translating new error messages from home-assistant#1906 to Swedish
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.