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 event for new list items #12

Merged

Conversation

alexschwantes
Copy link
Contributor

Now that the API has a single point of syncing, we can get onto the fun stuff 😃
I have been using this code for a couple weeks now and it is working pretty nicely.

Google recently removed third party integrations from Google Assistant. Now you can re-create these integrations.

When a new item is added to a synced list via Google Assistant or from Google Keep, a google_keep_sync_new_item event will be triggered. This allows Home Assistant to pick up new items from Google Keep and sync them with third party systems such as Trello, Bring, Anylist etc.

Note: Only new items that are not completed at the time of syncing will trigger the event.

@alexschwantes
Copy link
Contributor Author

hmm, I feel like ruff and black are fighting with eachother. Ruff complains about something, and the suggested fix causes Black to then complain, and suggest a fix, which then causes Ruff to complain lol.

Anyway, warning is fixed.

custom_components/google_keep_sync/__init__.py Outdated Show resolved Hide resolved
custom_components/google_keep_sync/__init__.py Outdated Show resolved Hide resolved
custom_components/google_keep_sync/__init__.py Outdated Show resolved Hide resolved
custom_components/google_keep_sync/__init__.py Outdated Show resolved Hide resolved
custom_components/google_keep_sync/__init__.py Outdated Show resolved Hide resolved
custom_components/google_keep_sync/__init__.py Outdated Show resolved Hide resolved
@watkins-matt
Copy link
Owner

Nice job with all this, definitely seems very useful. Just had some minor feedback/change requests on a few things I noticed.

@watkins-matt watkins-matt added the enhancement New feature or request label Jan 31, 2024
@alexschwantes
Copy link
Contributor Author

So I've managed to fix some of the existing tests, however I need some help with the _test_init.py tests. I still can't debug tests in windows or wsl2 :(

@watkins-matt
Copy link
Owner

I'll see if I can take a look at them.

So what I did to develop on Windows was followed the instructions here to set up a devcontainer with VS Code. Then I cloned the repository outside the Home Assistant folder, and then used ln to link custom_components/google_keep_sync into /workspaces/home-assistant-core/config/custom_components/google_keep_sync. Running tests and even debugging with breakpoints works for me with it configured like this.

@watkins-matt
Copy link
Owner

Everything looks good, tests should work now.

Quick question before I merge this in: is it intended that the event only fire when items are added in Google Keep, but not fire when they are added in Home Assistant?

That was the only thing I noticed when I was testing that I wasn't sure about. Wouldn't the event need to fire when items are added in Home Assistant too so that everything is synced to third party systems correctly?

@alexschwantes
Copy link
Contributor Author

So what I did to develop on Windows was followed the instructions here to set up a devcontainer with VS Code. Then I cloned the repository outside the Home Assistant folder, and then used ln to link custom_components/google_keep_sync into /workspaces/home-assistant-core/config/custom_components/google_keep_sync. Running tests and even debugging with breakpoints works for me with it configured like this.

Yes this is what I did as well for writing the code. I have this code checked out in a different folder and then linked the google_keep_sync folder into home assistant's custom components folder. But the unit tests are not brought across when doing this as they are outside of the google_keep_sync folder.. I can run the unit tests separately in their own vscode window, but can't debug them.

Quick question before I merge this in: is it intended that the event only fire when items are added in Google Keep, but not fire when they are added in Home Assistant?

Yes, this is how it works at present. It was intended to fill a hole within home assistant where home assistant fires an event only if the item is added to the todo list locally. It doesn't recognise updates from the remote list.

I catered for this by adding the following to the local automation to also pick up local additions to a list.

trigger:
  - platform: event
    event_type: call_service
    event_data:
      domain: todo
      service: add_item
    variables:
      list_name: "{{ state_attr((trigger.event.data.service_data.entity_id)[0], 'friendly_name') }}"
      item_name: "{{ trigger.event.data.service_data.item }}"
condition:
  - condition: template
    value_template: "{{ list_name == 'Google mylist' }}"

Hmmm.. the alternative is of course to fire the same event 'add_item' for the remotely added item to google keep list. On the add_item event that is fired locally, I can see that there is an origin which can be set as REMOTE for coming in from the API, e.g. a webhook, or LOCAL for everything else. I can't see much else mentioned about the origin field, but it could work...

  event:
    event_type: call_service
    data:
      domain: todo
      service: add_item
      service_data:
        item: new todo item
        entity_id:
          - todo.google_keep_mylist
    origin: LOCAL
    time_fired: '2024-02-05T00:38:21.662271+00:00'
    context:
      id: 01HNVB79PY1DASDASD
      parent_id: null
      user_id: a9e061e8f8074a1d8bc4f180ASDd9

@watkins-matt
Copy link
Owner

Not sure if it helps with the debugging issue, but I have the root google_keep_sync folder added as folder to the workspace. So my workspace has the google_keep_sync root folder and the home-assistant-core. So something like

	"folders": [
		{
			"name": "google_keep_sync",
			"path": "."
		},
		{
			"name": "home-assistant-core",
			"path": "../.."
		}
	],

within my keep-sync.code-workspace. VS Code picks up all the tests from the root directory and lets me debug.

As far as the event goes, I guess it is fixable with the second automation to pick up the local additions, but I think it would be more user friendly if we could potentially make it so the user doesn't have to make two seperate automations to pick up the additions.

The idea that you had might definitely work to just refire the same event, although I haven't looked into whether that would affect anything. Not sure if integrations are supposed to fire events from other integrations?

The other solution I was thinking that might work is maybe we do self.hass.bus.async_listen to listen for EVENT_CALL_SERVICE (filtering for todo events), and then use that data to refire google_keep_sync_new_item each time we pick up a todo.add_item. That way we don't mess with the existing event and we can ensure that every item comes through with the google_keep_sync_new_item. Then it would only take a single automation listening to google_keep_sync_new_item because it would catch all new items added.

@alexschwantes
Copy link
Contributor Author

ok So I got both methods working locally :)
So the two best options are:

  1. Fire an event the same as Home Assistant does, but with origin:REMOTE for items added in google keep.
    I have actually seen some discussion online where people were asking for this functionality in Home Assistant as it is how it is expected to behave. I can't find the links anymore, but this is what motivated me to do this functionality. It is a pretty clean solution, and it hasn't caused any adversed issues in my testing.

  2. Fire an google_keep_sync_new_item event for items added via Home Assistant (in addition to it being fired when items are added remotely). This does result in two events being called when a new item is added to HA locally as HA will emit its own event. The easy way to do this was to fire the event within the todo.async_create_todo_item() method (no need to listen to events).
    The origin for the remote addition of events could be set to REMOTE to differentiate between items added remotely and items added locally.
    eg.

            await self._handle_new_items_added(
                original_lists,
                updated_lists,
                self.config_entry.data.get("list_prefix", ""),
                lambda item_data: self.hass.bus.async_fire(
                    EVENT_NEW_ITEM, item_data._asdict(), origin=EventOrigin.remote
                ),
            )

On a side note:

I was looking more into the data being sent by the google_keep_sync_new_item event vs the simplified data being sent by HA's event. And I'm thinking that it might be better to keep it simple, with just the item name and list entity_id (sending the entity_id also makes filtering in automations easier than the list_name, and given the entity_id, the list name can be derived via state_attr((trigger.event.data.service_data.entity_id)[0],'friendly_name')).

This code currently sends a lot of data and I'm not sure how you would even use the gkeep list id, or the gkeep item id is of much use within HA. Guess I was just thinking more is better initially, but I'm feeling the opposite now, until someone has a specific need for it, I think it might be easier to simplify it.

            item_data = TodoItemData(
                item_name=item.summary,
                google_item_id=item.uid,  # when called from `todo.async_create_todo_item()`, this is null as it isn't in google yet.
                item_checked=item.status == TodoItemStatus.COMPLETED,
                list_name=self._attr_name,
                google_list_id=self._gkeep_list_id,
                list_entity_id=self.entity_id,
            )

result

Out of the two options, my preference is leaning towards option 1. But I thought I'd see what you thought. Either way works in the end though, so let me know your thoughts and I'll push the code :)

@watkins-matt
Copy link
Owner

I'm good with option 1, it does seem like a cleaner solution.

@watkins-matt watkins-matt merged commit 41223c8 into watkins-matt:main Feb 6, 2024
5 checks passed
@watkins-matt
Copy link
Owner

Thank you again, merged in and dropped a new version.

Just a heads up that I wanted to keep the default (visible) entity id the same for now (set in the todo constructor as self.entity_id), to try to avoid conflicts and make the entities easy to filter. Was there a specific reason you wanted to remove that part? Users can still change the id afterward manually.

@alexschwantes
Copy link
Contributor Author

alexschwantes commented Feb 6, 2024

Ah yes, sorry I meant to write a comment about that, but it got late and I forgot...

The code is completely unnecessary and in fact introduces a bug.

If two lists are created from two different accounts with the same title. ie Shopping List, there will be a collision of entity id as they both can't be called todo.google_keep_shopping_list. in reality, one should be called todo.google_keep_shopping_list_2

    def _get_default_entity_id(self, title: str) -> str:
        """Return the entity ID for the given title."""
        return f"todo.google_keep_{title.lower().replace(' ', '_')}"

As we set the self._attr_name, then home assistant will automatically create the correct entity_id with the method async_generate_entity_id() the entity class

https://github.com/home-assistant/core/blob/61ce328ce1866c7e307dde503cbea0868c27397b/homeassistant/helpers/entity.py#L118

where it has the default format as described as ENTITY_ID_FORMAT = DOMAIN + ".{}" in todo.__init.py

https://github.com/home-assistant/core/blob/61ce328ce1866c7e307dde503cbea0868c27397b/homeassistant/components/todo/__init__.py#L54

So the end result is that Home Assistant does this already for you, and appends an integer to the end for cases where there is a collision.

ps. Thanks for the help getting this through, I think its much better now that the same event is fired regardless of where the new item was added to the list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants