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

fix some details in appguide #1630

Merged
merged 1 commit into from
Feb 1, 2023
Merged

fix some details in appguide #1630

merged 1 commit into from
Feb 1, 2023

Conversation

moltob
Copy link
Contributor

@moltob moltob commented Jan 22, 2023

First of all: Thanks a lot for this doc page, it is a true jewel and exactly what I was missing in the jungle of docs and tutorials when approaching AD!

While reading and playing I found two leftovers from earlier AD versions I fixed.

@acockburn
Copy link
Member

Hi there - we really appreciate your efforts here, but unfortunately the corrections are not themselves correct. The import statement as shown is correct (AppDaemon adds the paths for the plugins to its module search path on startup), and set_app_state() is not a typo, it really is it's own API call.

If you disagree, and are having issues, by all mean post your errors here and we will get to the bottom of it.

@moltob
Copy link
Contributor Author

moltob commented Jan 24, 2023

Thanks for your consideration. Let me try to explain how I got there.

set_app_state was not offered by my IDE's code completion, so I started digging. If I search this repo, I only find (I assumed outdated) examples and this deprecation note for 4.0.0.b (it was "readded" in 3.0.0 before):

- ``set_app_state()`` is deprecated - use ``set_state()`` instead and it should do the right thing

Regarding the import: You're correct. However, I do use full import paths for IDE code completion and seem to remember the original helloworld example in the HA addon was also using a full import path. So I got the impression this was changed at some point.

I can update the PR to revert the imports if you confirm my first finding. Otherwise, I'd be happy to hear what I am missing and then please feel free to close the PR.

@acockburn
Copy link
Member

OK, good, you are right about set_app_state(), I remember now, it's been a while - thanks for catching that.

The imports are behaving as intended, it was a design decision to simply the sandbox and avoid arcane and nonsensical import statements. Of course there is no harm in doing it the way you suggest, it just wasn't what I was going for.

So, yep, go ahead and revert the imports and leave the fix for set_app_state() - thanks!

@moltob
Copy link
Contributor Author

moltob commented Jan 25, 2023

Commit with imports was dropped.

@acockburn
Copy link
Member

Thanks

@acockburn acockburn merged commit 08dd6a6 into AppDaemon:dev Feb 1, 2023
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.

2 participants