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

Use recommended format for Home Assistant MQTT sensor configuration #3344

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Use recommended format for Home Assistant MQTT sensor configuration #3344

merged 1 commit into from
Nov 29, 2023

Conversation

t3hk0d3
Copy link
Contributor

@t3hk0d3 t3hk0d3 commented Sep 15, 2023

Change documentation to offer example configuration using more robust format for MQTT sensors.
Proposed format is considered best practice by Home Assistant documentation

Additionally it introduces device grouping and adds sensor availability settings.

Dulanic
Dulanic reviewed Nov 13, 2023
View reviewed changes
Copy link
Collaborator

@Dulanic Dulanic left a comment

Choose a reason for hiding this comment

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

I like this, but why did you name everything teslamate_1? Either way, I dont have time to review right now, but I do like the idea behind it at least.

@t3hk0d3
Copy link
Contributor Author

t3hk0d3 commented Nov 15, 2023 •
edited
Loading

@Dulanic teslamate_1 is used as an example because teslamate exposes car data in teslamate/cars/1/<attribute> topic. In fact it could be named anything.

@JakobLichterfeld JakobLichterfeld added the kind:documentation Adds or improves documentation label Nov 16, 2023
@t3hk0d3
Copy link
Contributor Author

t3hk0d3 commented Nov 23, 2023

Rebase against lastest master branch, added TPMS sensor information

@t3hk0d3 t3hk0d3 requested a review from Dulanic November 23, 2023 23:09
@t3hk0d3
Copy link
Contributor Author

t3hk0d3 commented Nov 26, 2023

@JakobLichterfeld can you pls check?

@JakobLichterfeld
Copy link
Collaborator

Thanks for rebasing. teslamate-1 as name does not seem intuitive to me.

@t3hk0d3
Copy link
Contributor Author

t3hk0d3 commented Nov 26, 2023 •
edited
Loading

Thanks for rebasing. teslamate-1 as name does not seem intuitive to me.

@JakobLichterfeld are you talking about unique_id?
It is an internal identifier, and it won't be used for entity_id generation. https://www.home-assistant.io/integrations/sensor.mqtt/#unique_id

It is needed to group entities into device:

https://www.home-assistant.io/integrations/sensor.mqtt/#device

Only works when unique_id is set.

I can also add object_id to generate nice entity_id, such as tesla_charge_level

@JakobLichterfeld
Copy link
Collaborator

I can also add object_id to generate nice entity_id, such as tesla_charge_level

That would be great!

@t3hk0d3 t3hk0d3 force-pushed the patch-1 branch 2 times, most recently from b79fa3a to c534b26 Compare November 28, 2023 11:08
@t3hk0d3
Copy link
Contributor Author

t3hk0d3 commented Nov 28, 2023

@JakobLichterfeld I've updated PR. Please check again.

JakobLichterfeld
JakobLichterfeld approved these changes Nov 28, 2023
View reviewed changes
Copy link
Collaborator

@JakobLichterfeld JakobLichterfeld left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm, ty. One small typo finding

@JakobLichterfeld JakobLichterfeld self-requested a review November 28, 2023 12:50
@netlify Netlify
Copy link

netlify bot commented Nov 29, 2023 •
edited
Loading

✅ Deploy Preview for teslamate ready!

Name Link
🔨 Latest commit dca9e30
🔍 Latest deploy log https://app.netlify.com/sites/teslamate/deploys/656700e8889d3f00083b7405
😎 Deploy Preview https://deploy-preview-3344--teslamate.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

JakobLichterfeld
JakobLichterfeld approved these changes Nov 29, 2023
View reviewed changes
Copy link
Collaborator

@JakobLichterfeld JakobLichterfeld left a comment

Choose a reason for hiding this comment

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

lgtm

@JakobLichterfeld JakobLichterfeld merged commit abd1e6c into teslamate-org:master Nov 29, 2023
10 checks passed
@JakobLichterfeld JakobLichterfeld mentioned this pull request Nov 29, 2023
Closed
@t3hk0d3 t3hk0d3 mentioned this pull request Nov 29, 2023
Closed
1 task
@t3hk0d3 t3hk0d3 deleted the patch-1 branch November 29, 2023 14:18
JakobLichterfeld pushed a commit that referenced this pull request Feb 9, 2024
@t3hk0d3 @JakobLichterfeld
Use recommended format for Home Assistant MQTT sensor configuration (#…
153ff58 
…3344)
@longzheng longzheng mentioned this pull request Nov 3, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JakobLichterfeld JakobLichterfeld JakobLichterfeld approved these changes

@Dulanic Dulanic Awaiting requested review from Dulanic

Assignees
No one assigned
Labels
kind:documentation Adds or improves documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@t3hk0d3 @JakobLichterfeld @Dulanic