Skip to content

MQTT Fixes from Abstract#571

Merged
MikeBishop merged 20 commits intomainfrom
mqtt_data
Mar 6, 2025
Merged

MQTT Fixes from Abstract#571
MikeBishop merged 20 commits intomainfrom
mqtt_data

Conversation

@MikeBishop
Copy link
Copy Markdown
Collaborator

This PR attempts to pull out all the MQTT-related changes from #483, other than #492. This without #492 might be somewhat inconsistent.

Draft because I haven't explicitly tested this yet, just done the work in git to lift it out.

@MikeBishop MikeBishop marked this pull request as draft March 16, 2024 06:56
@RichieB2B
Copy link
Copy Markdown

I took a quick look and these changes are not compatible with paho mqtt client v2 and the MQTTv5 protocol. So this PR will probably only work for installs that are still using paho mqtt client v1.

I will fix this when I have more time.

@RichieB2B
Copy link
Copy Markdown

It turned out to be an easy fix, see RichieB2B@69027ca and the paho migration documentation for reference.

@MikeBishop
Copy link
Copy Markdown
Collaborator Author

Incorporated -- thank you!

@ngardiner
Copy link
Copy Markdown
Owner

Looks good, thanks @MikeBishop and @RichieB2B

@MikeBishop MikeBishop marked this pull request as ready for review May 15, 2024 21:12
@MikeBishop
Copy link
Copy Markdown
Collaborator Author

I tried this merged into my working branch (main + some PRs) and it seems to be working. Appreciate it if someone else can give it a try before we merge.

@RichieB2B
Copy link
Copy Markdown

RichieB2B commented Jan 9, 2025

There is a small conflict in this branch for the definition of update_location() and update_vehicle_data() in the current main. This is easily fixed with a rebase.

There is a small issue with the drive_state which I fixed in RichieB2B@225796d

When I run this code, it polls vehicle_data every minute for a car that is driving close to home. Can we set this to every 5 minutes? See RichieB2B@2a29ac6

@RichieB2B
Copy link
Copy Markdown

When a car is charging, update_vehicle_data() is updating vehicle_data every minute. I think this is too ofter (and too expensive) and every 5 minutes would suffice here as well.

@RichieB2B
Copy link
Copy Markdown

@MikeBishop Can you rebase to main and address the above issues? Seems ready to merge after that.

@MikeBishop
Copy link
Copy Markdown
Collaborator Author

How's that?

@RichieB2B
Copy link
Copy Markdown

Looks good to me!

@MikeBishop MikeBishop mentioned this pull request Jan 13, 2025
@MikeBishop
Copy link
Copy Markdown
Collaborator Author

And as an added benefit, the names do match up automatically, because of the 30-second delay on setting up MQTT. We've already found the display_name from the API before we get the first events from the MQTT server.

@MikeBishop
Copy link
Copy Markdown
Collaborator Author

@ngardiner, @RichieB2B any objections to merging?

@RichieB2B
Copy link
Copy Markdown

I've been running this code for 3 weeks now. Looks ready to merge to me.

@MikeBishop MikeBishop merged commit deb3a6b into main Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants