Skip to content

Add water level#853

Closed
killi199 wants to merge 12 commits into
denysdovhan:mainfrom
killi199:water-level
Closed

Add water level#853
killi199 wants to merge 12 commits into
denysdovhan:mainfrom
killi199:water-level

Conversation

@killi199

Copy link
Copy Markdown

Add a drop down menu for the water level.

Continuation / Rebase of #412

@killi199

Copy link
Copy Markdown
Author
image

@roumano

roumano commented Jan 17, 2025

Copy link
Copy Markdown

i would love to have this feature in the vacuum-card
@killi199 , it's still work in progress or it's ready to use and just waiting to be accepted to merge ?

@killi199

Copy link
Copy Markdown
Author

@roumano the feature is ready to use and is just waiting to be accepted for merging

@roumano

roumano commented Jan 18, 2025

Copy link
Copy Markdown

@denysdovhan , the feature is ready, can you have a look and accept to merge ?

@roumano

roumano commented Jan 31, 2025

Copy link
Copy Markdown

@denysdovhan , Can we help you on this repository to review/manage merge request or ...

@denysdovhan denysdovhan left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for contributing! This is a nice feature. Please address my comments and let's proceed with merging this feature.

Comment thread package.json Outdated
"home-assistant-js-websocket": "^9.2.1",
"lit": "^2.0.0",
"lodash": "^4.17.21"
"lodash": "^4.17.21",

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

As I see lodash is not used anywhere.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's used at line 10

Comment thread package.json Outdated
Comment thread src/editor.ts Outdated
Comment thread src/editor.ts Outdated
Comment thread src/vacuum-card.ts Outdated
Comment thread src/vacuum-card.ts
Comment on lines +121 to +113
return (
this.hass &&
!!this.config.water_level &&
changedProps.get('hass').states[this.waterLevelEntity].state !==
this.waterLevel.state
);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Doesn't hasConfigOrEntityChanged do the same thing?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No it does not. If this is missing the waterLevel only is updated when e.g. the fan speed was changed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I just noticed that this behavior is exactly the same for the stats (I've included them via entity_id instead of attribute). Those are only refreshed if the page is refreshed. I think this is the reason why this workaround is necessary.

I personally think that only the values are instantly refreshed that are directly in the vacuum entity.

Comment thread src/vacuum-card.ts Outdated
Comment thread src/vacuum-card.ts
${sources.map(
(item, index) => html`
${objects.map(
(item: string, index: number) => html`

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

TS should infer type here just fine. No need to be that specific.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Somehow it's not:

Parameter 'item' implicitly has an 'any' type.

@killi199

Copy link
Copy Markdown
Author

@denysdovhan sorry that it took me so long to address those comments. Now it is ready for review again. Thanks!

@killi199 killi199 requested a review from denysdovhan February 13, 2025 18:49
@clarinetJWD

Copy link
Copy Markdown

I merged this (and all the other PRs) into a build on my repository (PR #903) while the author is inactive. Feel free to use that version! (Note that I intend for this to be temporary so that you get your contribution credit here when the time comes)

@roumano

roumano commented Mar 14, 2025

Copy link
Copy Markdown

Thank you @clarinetJWD , it's working as expected :
HA - vacuum card  - water level

@github-actions

Copy link
Copy Markdown

This PR is stale because there hasn't been any activity for a long time.

@github-actions github-actions Bot added the stale label Jun 13, 2025
@killi199

Copy link
Copy Markdown
Author

Still not merged yet.

@github-actions github-actions Bot removed the stale label Jun 14, 2025
@github-actions

Copy link
Copy Markdown

This PR is stale because there hasn't been any activity for a long time.

@github-actions github-actions Bot added the stale label Sep 12, 2025
@github-actions

Copy link
Copy Markdown

This PR was closed because it has been stalled for 30 days with no activity.

@github-actions github-actions Bot closed this Oct 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants