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

Update connectors.md #3326

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Update connectors.md #3326

merged 1 commit into from
Oct 2, 2024

Conversation

supaeasy
Copy link
Contributor

Fixed errors in description:

  • add / after api
  • omit 'todo.' when adding HA-Entity

Fixed errors in description:
- add / after api
- omit 'todo.' when adding HA-Entity
@CLAassistant
Copy link

CLAassistant commented Sep 30, 2024

CLA assistant check
All committers have signed the CLA.

@vabene1111
Copy link
Collaborator

thank you for the update!

@vabene1111 vabene1111 merged commit f84a401 into TandoorRecipes:develop Oct 2, 2024
1 check passed
@supaeasy
Copy link
Contributor Author

supaeasy commented Oct 2, 2024

I am sorry but this PR might have been unneccessary if not even wrong. There is a problem authenticating in HA that I didn't know about at the time of creating the PR, see #911 (comment) and home-assistant/core#114575 so maybe there was some authentication eventually by chance but I am not sure which of my tries did the trick since sync seems to have been massively delayed. At least my first correction, adding "/" after "/api" is not needed (but not wrong either) since there is a line in the code adding it automatically if it isnt there. So yeah, maybe I should have gone slower with that. Not sure about omitting "todo."

@vabene1111
Copy link
Collaborator

@mikhail5555 can you look into if this should be reverted ?

@mikhail5555
Copy link
Contributor

I would indeed recomment reverting it. I am pretty sure todo. should not be emited since this is the 'target' entity of the request in HomeAssistant.
And indeed, the / behind api is added automatically, so no need to include it.

@supaeasy
Copy link
Contributor Author

supaeasy commented Oct 2, 2024

Sorry for the hassle!

@mikhail5555
Copy link
Contributor

Sorry for the hassle!

No worries, its always appreciated when people keep the documentation up to date. Mistakes happen.

Also, I am using this weekly and am not having any authentication issues. If you need some help hit me up on discord and I will try to help you out!

@vabene1111
Copy link
Collaborator

thank you for the feedback, going to revert this.

@supaeasy
Copy link
Contributor Author

supaeasy commented Oct 4, 2024

Thank you for your offer @mikhail5555 I will contact you once I am back from vacation, some 10ish days or so. Maybe you could look into my question here about deleting or checking items in the API in the meantime. I am still getting all of my imported recipes into structure but would like to go "productive" with one solution or the other - though I think my way would fit my needs better since it includes all the details I want for bring list.

@mikhail5555
Copy link
Contributor

Thank you for your offer @mikhail5555 I will contact you once I am back from vacation, some 10ish days or so. Maybe you could look into my question here about deleting or checking items in the API in the meantime. I am still getting all of my imported recipes into structure but would like to go "productive" with one solution or the other - though I think my way would fit my needs better since it includes all the details I want for bring list.

So from the APIs the only way to achieve what you are doing is indeed from Home assistants side because the api (not websocket) just doesn't expose the necessary info of that kind of synchronization (and would require to continually poll home assistant to get updates).
So indeed implementing it through hacs will likely be better (since TandoorRecipes api does allow for full management)

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

Successfully merging this pull request may close these issues.

4 participants