Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Feature/lawn mower entity #216

Merged
merged 2 commits into from
May 24, 2024
Merged

Feature/lawn mower entity #216

merged 2 commits into from
May 24, 2024

Conversation

dereisele
Copy link
Contributor

@dereisele dereisele commented May 19, 2024 •
edited
Loading

Add Lawn Mower Entity

Home Assistant has a native lawn mower entity since version 2023.9 so I quickly added it.
It is based on the vacuum entity. The state mapping reflects the husqvarna implementation.

image

@sander1988
Copy link
Owner

@dereisele - Thank you for your work! Makes sense to use the mower base entity here now it's available in HA.

Instead of coping everything, can you just change the vacuum to mower?

There is no reason anymore to keep the vacuum, we can drop that support now.

@dereisele
Copy link
Contributor Author

Hi, I would keep the vacuum entity for now, because it's supported by the Google Assistant integration, but the lawn mower isn't.

The largest non-specific copy-paste is the state mapping. Maybe it could be moved to the mix-in Indego Entity.

@sander1988
Copy link
Owner

I would keep the vacuum entity for now, because it's supported by the Google Assistant integration, but the lawn mower isn't.

Ok, that's a good argument to keep it to prevent breaking things. I think we should make it configurable... expose it as vacuum, mower or none.

I will have a look at your code soon and try to merge it.

@sander1988 sander1988 changed the base branch from develop to lawn-mower-entity-feature May 24, 2024 13:44
@sander1988
Copy link
Owner

I will be merging it in a separate branch for now. I want to make some additional changes before merging it in the main develop branch.

@sander1988 sander1988 merged commit fae783b into sander1988:lawn-mower-entity-feature May 24, 2024
@dereisele
Copy link
Contributor Author

Nice, comment here if you need any help

@sander1988
Copy link
Owner

It has been merged, optimized and made configurable. It's now on the develop branch.

Nice, comment here if you need any help
Can you test it?

Expose as 'mower' or 'vacuum' if now configurable. Both disabled by default. This can be enabled through the integration configuration panel.

@sander1988
Copy link
Owner

@wimb0 - I noticed you have added the PL translations some time ago. Some translations are now missing due to new functionality. Any chance you can add the new translations?

@wimb0
Copy link
Contributor

wimb0 commented May 27, 2024

Maybe @zmechu can help with the Polish translations. He is the one who did the translation.
@sander1988 Is Dutch already up-to-date, if not I can add these.?

@zmechu
Copy link

zmechu commented May 27, 2024

Maybe @zmechu can help with the Polish translations. He is the one who did the translation. @sander1988 Is Dutch already up-to-date, if not I can add these.?

Sure! Which file should be translated?

@wimb0
Copy link
Contributor

wimb0 commented May 27, 2024

I think this one: https://github.com/jm-73/Indego/blob/lawn-mower-entity-feature/custom_components/indego/translations/pl.json
Seems like EN (and NL) is already translated, so you can use https://github.com/jm-73/Indego/blob/lawn-mower-entity-feature/custom_components/indego/translations/en.json as source for what needs to be translated.

@zmechu
Copy link

zmechu commented May 27, 2024

I think this one: https://github.com/jm-73/Indego/blob/lawn-mower-entity-feature/custom_components/indego/translations/pl.json Seems like EN (and NL) is already translated, so you can use https://github.com/jm-73/Indego/blob/lawn-mower-entity-feature/custom_components/indego/translations/en.json as source for what needs to be translated.

Done, please take a look if all is OK.

@urbatecte
Copy link

urbatecte commented May 27, 2024 via email

@kimzeuner
Copy link
Contributor

Do you mean the alert description and headline ?
image
image

@urbatecte
Copy link

urbatecte commented May 27, 2024 via email

@kimzeuner
Copy link
Contributor

Ok, at the moment it is not possible because some codes have to be added first but im working on it and hope to finish it tomorrow. I will write anpther PR and let you know when im done so you can start working on your additional translations 😉

@urbatecte
Copy link

urbatecte commented May 27, 2024 via email

@urbatecte
Copy link

urbatecte commented May 27, 2024 •
edited
Loading

@kimzeuner
I'm not used to make PR :-(
Please find here attached my translation for french
Thanks a lot for your job guys !

fr.json

@kimzeuner
Copy link
Contributor

Good morning guys,
i have added the translations for the entities "Alert Description", "Alert Headline", "Mower State" and "Mower State Detail".
Can you please have a look to the translation files here and change it where needed ?! i left the new lines, where you have to make your changes, blank so you only have to insert the correct translation between the " ".
@zmechu i have added the translation for the services in the pl.json file in the link above so they need to be translated too ;)

Please let me know when you have finished the translations and i will create the PR for that.

One additional request: While you are translating the texts, could you please have a look if the list of possible alerts and mower states is complete ? Unfortunately i have not found a complete list of all possible messages, that's why I could only use the ones that I have encountered so far.

Thanks a lot!

@urbatecte
Copy link

Hi @kimzeuner ,

Here is my french translation file.
fr.json

It seems that some alert message are missing.
In my app, I still have kind of:
Mower stopped by an obstacle
Check blades
Mower immobile (different status than mower stucked, wheel still running but fixed position)
Connection to docking station not possible
Mower too slopped/inclined

Cheers

@sander1988
Copy link
Owner

I think there is no need to translate alert messages. They are returned by the API in the language of the account. I just tested it, I get my alerts in my native language (Dutch).

What I see:

  • alert description = English
  • alert message = Native language

@sander1988
Copy link
Owner

And note that code (including alerts) is still being optimized and might change a bit before the release.

@kimzeuner
Copy link
Contributor

Correct, i saw the same (alert message in native language) in my system, thats why i have not translated them.
Should i wait with a PR for the translations until you have finished optimizations ?

@sander1988
Copy link
Owner

Correct, i saw the same (alert message in native language) in my system, thats why i have not translated them. Should i wait with a PR for the translations until you have finished optimizations ?

I recommend it, I'm nearly done optimizing the alert part, will commit it when complete. I will also open a separate issue for translations when ready. It's not really related to this PR, so a bit confusing.

@sander1988 sander1988 mentioned this pull request May 28, 2024
Closed
3 tasks
@sander1988
Copy link
Owner

Please provide all translation updates in a PR or file in issue #221. Thank you! Note that I just pushed an update to the develop branch requiring more translations ;-)

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dereisele @sander1988 @wimb0 @zmechu @urbatecte @kimzeuner