unfoldedcircle / feature-and-bug-tracker

Feature and bug tracker repository for Unfolded Circle products
24 stars 0 forks source link

[bug] activities made available (and selected) by integration can not start #392

Closed Albatross7591 closed 2 months ago

Albatross7591 commented 2 months ago

Is there an existing issue for this?

Description

Activities provided by an integration can not start (will hang starting). The screen of the remote will show '0 of 85 steps' (there were NOT 85 steps defined).

Screenshot of the remote starting an activity from an integration: Activity bug

How to Reproduce

  1. Create integration
  2. During setup integration responds on 'get_available_entities' with one (or more) entities of type 'activity'
  3. Select that specific entity
  4. Start activity

Expected behavior

Starting the activity will trigger the start of the activity (and the steps defined for start).

System version

v1.7.14

What part of the system affected by the problem?

Core, Integration

Additional context

No response

zehnm commented 2 months ago

An integration cannot provide activity entities. This is an internal entity type. If that somehow slips through the get_available_entities call, then that's the issue. Are you the developer of the integration?

Albatross7591 commented 2 months ago

An integration cannot provide activity entities. This is an internal entity type. If that somehow slips through the get_available_entities call, then that's the issue. Are you the developer of the integration?

Hoi Markus, I am the developer of the Harmony Hub integration. First: it slips through! I made activities available for selecting (so a user can copy Harmony Hub activities). The documentation https://studio.asyncapi.com/?url=https://raw.githubusercontent.com/unfoldedcircle/core-api/main/core-api/websocket/UCR2-asyncapi.yaml#message-available_entities lists that for entity type allowed values are: button, climate, cover, light, media_player, sensor, switch, activity, macro, remote (maybe that's needs updating ;-).

There are two reasons for doing this:

  1. deleting the integration will also delete these activities (no 'garbage left')
  2. performance: the only way to setup the UI is after selecting entities. Currently generating and activities, activity groups, button mappings , pages can take a full minute. An other thing is that this way a UI restart it is necessary to restart UI (icons are uploaded, but a UI restart is necessary to show them, maybe I should make a bug report of this separately). I noticed that using R2 to create activities (and updates them afterwards) is faster.

(to have an idea what I am trying to accomplish, have a look at the video I posted on the discord channel https://discord.com/channels/553671366411288576/1252344707934064761 )

zehnm commented 2 months ago

I'm sorry to be the bearer of bad news. You are referencing the Core-API where one can create and interact with all entities from the remote, including the internal macro- & activity-entities. For an integration driver however, the Integration-API is relevant what kind of entities can be provided as available entities. These can then be selected in the remote (https://github.com/unfoldedcircle/core-api/blob/v0.10.0-beta/integration-api/asyncapi.yaml#L1614-L2052). Macros & activities are also not listed in the entity documentation.

I will make that more explicit in the documentation and fix the available entity loading to only show supported entity types.

It is not possible to correctly associate macro- and activity-entities to an external integration driver. If it slips through get_available_entities and adding such entities to the remote, it will not work correctly or not at all. (E.g. what you describe in the opened bug reports). There is no simple way to fix it, since the design was never intended to link an activity to an external driver. It was designed to be independent of a driver and allow to combine various entities from different drivers. Changing this would require a major rewrite. Also, the activity execution is an internal logic and cannot be delegated to the driver. In the future there will be more logic to automatically map the buttons & UIs from multiple entities, plus you can select which device is responsible for volume control, which one for media control etc. I think it's dangerous to link an activity to a driver and automatically delete it with the driver. The normal use-case of an activity is to combine multiple sources/drivers. A user would have to keep in mind, that this is a special activity which might get deleted. Or we'd have to make the activity read-only or restricted to entities from that particular driver.

An integration driver can still create activities with the Core-API, but only after the dependent entities are configured in the remote.

What I could think about if there's demand from other integration drivers as well, is something like "activity templates": an integration driver provides the required data for an activity (but only for entities provided by the same driver), and the remote would create a real activity from that information. This would then be a regular, internal activity (and not linked to an external driver). The user can freely modify the activity afterwards and add additional entities from other drivers.

I don't fully understand your 2. reason. What is the performance issue? Doing the preparation in your integration, or API calls in the remote? Please explain the UI restart issue. If the UI page is currently open which is getting modified, then not all changes are shown instantly. The UI needs to be closed and re-opened.

Albatross7591 commented 2 months ago

Don't worry, I will closed this bug report (and the other 2 too).

For reason 2: it is more about user experience (waiting for a minute without having any indication what is going could be confusing: after selecting entities the webconfiguration notifies the user it is done, while actually the integration is setting up the UI elements)

Regarding the necessary restart of the UI, I will make a video and attach that to a new bug report.