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

Jump to bottom

Add function to list tracker entities and register them. #89

Merged
merged 9 commits into from Mar 17, 2022
Merged

Add function to list tracker entities and register them. #89

merged 9 commits into from Mar 17, 2022

Conversation

Tony763
Copy link

@Tony763 Tony763 commented Oct 24, 2021 •
edited

Description

Added function to get list of tracker entities at connection to HA.
List is saved as tracker.entity in vocab folder and registered by register_entity_file()

Padatious intent for tracker then react only for them.

Should solve conflict with skill where aka #64 .

Type of PR

  • Bugfix
  • Feature implementation
  • Refactor of code (without functional changes)
  • Documentation improvements
  • Test improvements

Testing

Added test vk test where skill should not react.

@Tony763 Tony763 force-pushed the feature/issue64 branch 7 times, most recently from 98b2ebe to f9e2428 Compare October 29, 2021 07:12
@Tony763
Copy link
Author

Tony763 commented Oct 29, 2021

Hi @krisgesling , PR rebased with latest changes. Could you start tests?

@krisgesling krisgesling added the secure Code has been reviewed and is safe to execute in Github Actions. label Nov 2, 2021
@github-actions github-actions bot removed the secure Code has been reviewed and is safe to execute in Github Actions. label Nov 2, 2021
krisgesling
krisgesling reviewed Nov 2, 2021
View reviewed changes
Copy link

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for tackling this issue. It will be great to update the Skill in the Marketplace.

Can we add in a couple of extra Skill should not reply tests just to be extra sure that this works and continues to work. Maybe something like:

  • where is France
  • where is the golden gate bridge

@Tony763
Copy link
Author

Tony763 commented Nov 2, 2021

Hi Kris, tests added and passing. Allure report

@krisgesling krisgesling added the secure Code has been reviewed and is safe to execute in Github Actions. label Nov 10, 2021
@github-actions github-actions bot removed the secure Code has been reviewed and is safe to execute in Github Actions. label Nov 10, 2021
krisgesling
krisgesling reviewed Nov 10, 2021
View reviewed changes
Copy link

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, excited to see this get in and restore the glory of HomeAssistant in the Marketplace!

I just had another look at the PR and noticed a few things that would be good to tidy up before we merge.

ha_client.py Show resolved Hide resolved
__init__.py Outdated Show resolved Hide resolved
test/behave/12_tracker.feature Outdated Show resolved Hide resolved
__init__.py Show resolved Hide resolved
__init__.py Show resolved Hide resolved
ha_client.py Outdated Show resolved Hide resolved
krisgesling
krisgesling reviewed Nov 12, 2021
View reviewed changes
__init__.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the workflow This PR contains changes to the Github workflow. They should be reviewed carefully. label Dec 7, 2021
@Tony763
Copy link
Author

Tony763 commented Dec 7, 2021

Wow, change in workflow correctly detected 🙂

@Tony763
Copy link
Author

Tony763 commented Dec 7, 2021

Fixed some weird merge issues, houp I did not break anything.

@krisgesling could You check changes, please? Also change in workflow is necessary.

@krisgesling krisgesling added the secure Code has been reviewed and is safe to execute in Github Actions. label Dec 8, 2021
@github-actions github-actions bot removed the secure Code has been reviewed and is safe to execute in Github Actions. label Dec 8, 2021
krisgesling
krisgesling reviewed Dec 8, 2021
View reviewed changes
Copy link

@krisgesling krisgesling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great - and thanks heaps for all the extra docstrings and type annotations!

test/behave/12_tracker.feature Outdated Show resolved Hide resolved
__init__.py Show resolved Hide resolved
@krisgesling
Copy link

This I believe will fix the installation issue:
MycroftAI/mycroft-core#3048

@Tony763
Copy link
Author

Tony763 commented Dec 8, 2021

Fix for action is in this PR. Check change in workflow file and action run in my repo.

pfefferle
pfefferle reviewed Dec 8, 2021
View reviewed changes
__init__.py Show resolved Hide resolved
@Tony763
Copy link
Author

Tony763 commented Dec 8, 2021

Ou, I just opened an allure report and there is something wrong, I will recheck it tonight.

@Tony763 Tony763 mentioned this pull request Dec 9, 2021
Merged
5 tasks
@Tony763
Copy link
Author

Tony763 commented Dec 9, 2021

@krisgesling fixed, should be ready to go 🙂

@github-actions github-actions bot removed the secure Code has been reviewed and is safe to execute in Github Actions. label Dec 13, 2021
@Tony763
Copy link
Author

Tony763 commented Dec 21, 2021

Hi @krisgesling , tests will fail until the fix for workflow in this PR is merged.
Should I send it as different PR?

@Tony763
Copy link
Author

Tony763 commented Jan 30, 2022

bump @krisgesling

@krisgesling krisgesling added the secure Code has been reviewed and is safe to execute in Github Actions. label Feb 28, 2022
@github-actions github-actions bot removed the secure Code has been reviewed and is safe to execute in Github Actions. label Feb 28, 2022
@krisgesling
Copy link

Sorry Tony, was out for a few months and its taken a while to catch up on things.

The overall test run seems to be failing on dev_setup. I'm doing a fix in core that might solve that.

It would be better if we did the workflow changes in a new PR - just easier to track what's happening

@krisgesling krisgesling added the secure Code has been reviewed and is safe to execute in Github Actions. label Mar 2, 2022
@github-actions github-actions bot removed the secure Code has been reviewed and is safe to execute in Github Actions. label Mar 2, 2022
pfefferle added a commit to pfefferle/skill-homeassistant that referenced this pull request Mar 13, 2022
@pfefferle
fix build process
3558fde 
Added the build changes by @Tony763 to fix the latest failures.

Requested by @krisgesling MycroftAI#89 (comment)
@pfefferle pfefferle mentioned this pull request Mar 13, 2022
Merged
5 tasks
@github-actions github-actions bot removed the workflow This PR contains changes to the Github workflow. They should be reviewed carefully. label Mar 16, 2022
@Tony763
Copy link
Author

Tony763 commented Mar 16, 2022

Hi @krisgesling, hope You are okay 🙂 I rebased PR with latest changes merged into 20.08. Every thing seems to work again so feel free to add secure label to start tests.

@krisgesling krisgesling added the secure Code has been reviewed and is safe to execute in Github Actions. label Mar 17, 2022
@github-actions github-actions bot removed the secure Code has been reviewed and is safe to execute in Github Actions. label Mar 17, 2022
@krisgesling
Copy link

Great, thanks :)

I'm doing well - had our second baby and then some covid thrown in for good measure 😵 but come out the other side and slowly getting back on top of all our repo's!

@krisgesling
Copy link

WOOOOH 🚀

@krisgesling krisgesling merged commit ebf5018 into MycroftAI:20.08 Mar 17, 2022
@Tony763
Copy link
Author

Tony763 commented Mar 17, 2022

Great, thanks :)

I'm doing well - had our second baby and then some covid thrown in for good measure dizzy_face but come out the other side and slowly getting back on top of all our repo's!

Then congratulation 🎉. Yeah, covid is spreading there too, I got it two weeks ago. Thankfully, vaccination worked so I managed it with just light cold. Just long stairs are now enemy number one, but it gets better each day.

@zeronounours zeronounours mentioned this pull request Apr 24, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pfefferle pfefferle pfefferle left review comments

@krisgesling krisgesling krisgesling approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
PR Processing
Status: Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Tony763 @krisgesling @pfefferle