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

Publish extra active_route fields to mqtt output #3789

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

brianmay
Copy link
Collaborator

This refactors the existing code to make it more maintainable.

Nil values are published as "nil" string. This ensures that they will get published, and ensures that MQTT doesn't drop the retained data.

This is WIP because it hasn't had any testing yet, and is likely to fail even the CI tests :-)

Fixes #3748

Copy link

netlify bot commented Mar 28, 2024

Deploy Preview for teslamate ready!

Name Link
🔨 Latest commit 3cbabf9
🔍 Latest deploy log https://app.netlify.com/sites/teslamate/deploys/660a03dfb172b50008380c2d
😎 Deploy Preview https://deploy-preview-3789--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.

@brianmay
Copy link
Collaborator Author

On my box I am seeing 5 test failures. I imagine I broke something, but don't have time to check right now.

@brianmay
Copy link
Collaborator Author

Looks like there is an error in the documentation also.

@brianmay
Copy link
Collaborator Author

Looks like CI tests agree with my tests, there are 5 tests failing. Looks like the same tests are failing also.

@brianmay
Copy link
Collaborator Author

Tests now passing. Seems to be a problem with building the docs.

@brianmay brianmay force-pushed the active_route_improvements branch 2 times, most recently from 2d38ba9 to d9742e5 Compare March 29, 2024 22:13
@brianmay
Copy link
Collaborator Author

Made some changes.

  • There is now an active_route topic which publishes all active route information in one json blob. I consider all of these connected and should update atomically.

  • When publishing the json blobs, if there is an error, we will output that as a json value too. Meaning value should always be json, should make it easier to parse.

Wondering if we still need the active_route_* values now, or if active_route by itself is sufficient. Not fussed, but this could be simplified a little bit if I deleted the active_route_* values and everything would still be published.

@JakobLichterfeld JakobLichterfeld added the enhancement New feature or request label Mar 30, 2024
@longzheng
Copy link
Contributor

longzheng commented Mar 31, 2024

RE: changes to reset active_route_ topics when not actually routing
I'm not familiar with Elixer at all but this look reasonable to check active_route_destination: nil to reset all the active_route_ topics to nil.

RE: adding extra active_route topics
Thanks for adding all the other topics I didn't bother to do active_route_energy_at_arrival active_route_miles_to_arrival active_route_minutes_to_arrival active_route_traffic_minutes_delay

RE: adding JSON topics
It's my understanding that you're adding two JSON-blob topics active_route and active_route_location. The existing separate topics are still there. This also looks/sounds reasonable to me.

I'm not sure if the goal is to eventually deprecate the separate topics, if so we should update the MQTT and Home Assistant (although I just realised I forgot to update this for the active route sensors) documentations.

@brianmay
Copy link
Collaborator Author

I just removed the new non-json topics in favour of the json topics. And marked the old topics as deprecated in the docs.

@brianmay
Copy link
Collaborator Author

I haven't tested this yet - apart from the CI tests, I imagine that will be the next step :-)

@brianmay
Copy link
Collaborator Author

brianmay commented Apr 1, 2024

Dump of all published values on start, hopefully nothing is missing :-)

teslamate/cars/1/wheel_type AeroTurbine19
teslamate/cars/1/model S
teslamate/cars/1/geofence Home
teslamate/cars/1/active_route {"error":"No active route available"}
teslamate/cars/1/tpms_soft_warning_rr false
teslamate/cars/1/charging_state Stopped
teslamate/cars/1/tpms_soft_warning_fr false
teslamate/cars/1/active_route_destination nil
teslamate/cars/1/longitude xxx
teslamate/cars/1/charger_power 0
teslamate/cars/1/doors_open false
teslamate/cars/1/since 2024-03-31T23:11:18.410000Z
teslamate/cars/1/charge_current_request 32
teslamate/cars/1/exterior_color Red
teslamate/cars/1/locked true
teslamate/cars/1/power 0
teslamate/cars/1/version 2024.2.7
teslamate/cars/1/shift_state (null)
teslamate/cars/1/plugged_in true
teslamate/cars/1/tpms_pressure_fr 3.375
teslamate/cars/1/active_route_longitude nil
teslamate/cars/1/charger_actual_current 0
teslamate/cars/1/windows_open false
teslamate/cars/1/scheduled_charging_start_time (null)
teslamate/cars/1/ideal_battery_range_km 333.6
teslamate/cars/1/healthy true
teslamate/cars/1/charge_limit_soc 80
teslamate/cars/1/spoiler_type None
teslamate/cars/1/is_preconditioning false
teslamate/cars/1/tpms_soft_warning_rl false
teslamate/cars/1/est_battery_range_km 249.42
teslamate/cars/1/frunk_open false
teslamate/cars/1/trim_badging 90D
teslamate/cars/1/tpms_pressure_fl 3.375
teslamate/cars/1/trunk_open false
teslamate/cars/1/is_user_present false
teslamate/cars/1/usable_battery_level 80
teslamate/cars/1/charge_energy_added 0.0
teslamate/cars/1/charger_phases (null)
teslamate/cars/1/tpms_pressure_rl 3.425
teslamate/cars/1/odometer 158835.27
teslamate/cars/1/location {"latitude":xxx,"longitude":xxx}
teslamate/cars/1/charge_port_door_open true
teslamate/cars/1/latitude xxx
teslamate/cars/1/tpms_pressure_rr 3.4
teslamate/cars/1/charge_current_request_max 32
teslamate/cars/1/time_to_full_charge 0.08
teslamate/cars/1/update_available false
teslamate/cars/1/rated_battery_range_km 415.89
teslamate/cars/1/inside_temp 23.9
teslamate/cars/1/heading 13
teslamate/cars/1/state online
teslamate/cars/1/tpms_soft_warning_fl false
teslamate/cars/1/outside_temp 21.0
teslamate/cars/1/charger_voltage 0
teslamate/cars/1/climate_keeper_mode off
teslamate/cars/1/active_route_latitude nil
teslamate/cars/1/is_climate_on false
teslamate/cars/1/battery_level 80
teslamate/cars/1/update_version (null)

I censored latitude, longitude; I don't particularly want to publish the location of my home.

Next step will be to dump the values when navigiating.

@brianmay brianmay changed the title WIP: Publish extra active_route fields to mqtt output Publish extra active_route fields to mqtt output Apr 1, 2024
This refactors the existing code to make it more maintainable.

Nil values are published as "nil" string. This ensures that they will
get published, and ensures that MQTT doesn't drop the retained data.

Fixes #3748
@brianmay
Copy link
Collaborator Author

brianmay commented Apr 4, 2024

Finally got around to looking at the new topic when car is navigating. Everything looks good to me.

If no objections, will merge tomorrow.

@brianmay brianmay merged commit c684252 into master Apr 5, 2024
14 checks passed
@brianmay brianmay deleted the active_route_improvements branch April 5, 2024 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MQTT values for active_route_* are not clearing
3 participants