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.

Sign up for GitHub

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

Added 030-1wk100028v0w (Hisense DH5S102BW tumble dryer) #120

Merged
merged 11 commits into from
Oct 22, 2024
Merged

Added 030-1wk100028v0w (Hisense DH5S102BW tumble dryer) #120

merged 11 commits into from
Oct 22, 2024

Conversation

gregrs-uk
Copy link
Contributor

Started mapping Hisense DH5S102BW tumble dryer

@gregrs-uk gregrs-uk mentioned this pull request Aug 23, 2024
Closed
oyvindwe
oyvindwe requested changes Aug 24, 2024
View reviewed changes
Copy link
Owner

@oyvindwe oyvindwe left a comment

Choose a reason for hiding this comment

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

Thank your for your contribution! ❤️

I added some comments even though you set it as a draft.

custom_components/connectlife/data_dictionaries/030-1wk100028v0w.yaml Outdated Show resolved Hide resolved
custom_components/connectlife/data_dictionaries/030-1wk100028v0w.yaml Show resolved Hide resolved
custom_components/connectlife/strings.json Outdated Show resolved Hide resolved
custom_components/connectlife/strings.json Outdated Show resolved Hide resolved
@gregrs-uk
Copy link
Contributor Author

I still need to work out the values for Current_program_phase but @oyvindwe would you mind taking a look at my latest commits?

@oyvindwe
Copy link
Owner

@gregrs-uk Looks good to me so far!

@gregrs-uk gregrs-uk marked this pull request as ready for review October 21, 2024 12:42
@gregrs-uk
Copy link
Contributor Author

@oyvindwe There are other properties available but I've defined the most useful ones so I think that's as far as I'm likely to go. Would you mind checking and merging if you're happy?

@gregrs-uk
Copy link
Contributor Author

Sorry, just spotted an error from when I merged the main branch into this feature branch. Will fix…

@gregrs-uk
Copy link
Contributor Author

@oyvindwe Should be good to go now. I've rebased on main so should be easy to merge.

@gregrs-uk
Amalgamate [Cc]urrent_program_phase states
b3943bb 
Looks like property names in strings.json are case-insensitive, so shouldn't supply a separate Current_program_phase property
@gregrs-uk
Copy link
Contributor Author

gregrs-uk commented Oct 21, 2024 •
edited
Loading

Any idea why properties aren't picking up the names I've given them in strings.json and en.json? For example 'AntiCrease' should be 'Anti crease'. (Others have the wrong capitalisation.) I've tried restarting Home Assistant and removing and re-adding the integration.

Screenshot 2024-10-21 at 21 09 12

@oyvindwe
Copy link
Owner

Any idea why properties aren't picking up the names I've given them in strings.json and en.json?

In strings.json and en.json, the keys must be in lowercase, while the mapping file uses the casing from the API. This was introduced in #138

It's stated in the doc, but a bit hidden:
https://github.com/oyvindwe/connectlife-ha/tree/main/custom_components/connectlife/data_dictionaries#mapping-tips-and-tricks

@oyvindwe
Copy link
Owner

If you use the python -m scripts.gen_strings script, it should generate the missing keys.

@gregrs-uk
Copy link
Contributor Author

Thanks. I will fix that. I didn't see python -m connectlife.dump --username <username> --password <password> --format dd in the docs originally so not all the properties are included in my mapping, only the ones I've defined. Would you like the properties I haven't defined to be included (but hidden)?

@gregrs-uk
Change order of properties to match dump output
c69daed
@gregrs-uk
Change strings to match output from gen_strings
bda5e4b 
Didn't remove machine_status: off or Dry_Level as suggested by gen_strings
@gregrs-uk
Copy link
Contributor Author

Hopefully ready to merge now

oyvindwe
oyvindwe reviewed Oct 22, 2024
View reviewed changes
custom_components/connectlife/translations/en.json Outdated
@@ -1439,6 +1480,14 @@
}
},
"select": {
"Dry_Level": {
Copy link
Owner

Choose a reason for hiding this comment

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

Great work! You missed this one though.

@oyvindwe
Copy link
Owner

Thanks! I fixed the remaining issues with the translation keys.

@oyvindwe oyvindwe merged commit 6581888 into oyvindwe:main Oct 22, 2024
1 check passed
@oyvindwe oyvindwe changed the title 030-1wk100028v0w (Hisense DH5S102BW tumble dryer) Added 030-1wk100028v0w (Hisense DH5S102BW tumble dryer) Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oyvindwe oyvindwe oyvindwe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gregrs-uk @oyvindwe