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

Add Send Destination function #207

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

DarkFox
Copy link

@DarkFox DarkFox commented Sep 13, 2024

* Add support for sending a route or destination
@tillsteinbach
Copy link
Owner

This is great, the problem I see is, how does that behave when used downstream. So far the controls don't use complex objects as an input.
When I merge this, in weconnect-mqtt there will ne a new topic, but it will not be usable. I need to think of a solution for that.

@DarkFox
Copy link
Author

DarkFox commented Sep 14, 2024

That's a good point.

Perhaps if we allow the value to be a dict or a list of dicts, and then have a class method in Route that creates the objects?

Something like:

...
if isinstance(value, list):
    Route.create_from_list(value)
...

That way the validation logic can stay in there and not gunk up controls.py

Edit: or allow a JSON string, which is then converted?

@tillsteinbach
Copy link
Owner

I like the json idea. This needs good testing but should be a nice solution.

@tillsteinbach tillsteinbach self-requested a review September 15, 2024 07:35
@DarkFox
Copy link
Author

DarkFox commented Sep 18, 2024

Would you prefer that I squash the commits before merging, or will you just do a squash and merge?

DarkFox added a commit to DarkFox/volkswagen_we_connect_id that referenced this pull request Sep 19, 2024
DarkFox added a commit to DarkFox/volkswagen_we_connect_id that referenced this pull request Sep 20, 2024
@DarkFox
Copy link
Author

DarkFox commented Oct 6, 2024

@tillsteinbach I was wondering if there is anything I can do to help move this forward? Are there any further code changes or testing that needs to be done?

I've been running this on my own Home Assistant instance for a few weeks now, and it's working great.

@tillsteinbach
Copy link
Owner

Sorry, it's just that I'm very busy. If you have a chance testing it with Weconnect-cli and Weconnect-mqtt, this is what is left for me to merge it.

@DarkFox
Copy link
Author

DarkFox commented Oct 6, 2024

I understand, it's no problem at all. I'll take a look at that ASAP. 👍

@DarkFox
Copy link
Author

DarkFox commented Dec 18, 2024

Apologies for the delay in finishing this PR. I've been dealing with some health issues that have impacted my ability to work on it.

I was in the process of trying to figure out the correct MQTT topic to use for sending the data, but I ran into some uncertainty there. Additionally, I'm not sure I had the CLI fully operational, which further complicated things.

Unfortunately, I don't currently have the bandwidth to finish this. If anyone else is interested in picking it up, I'd be happy to provide context on what I've done so far.

Thanks for understanding!

@tillsteinbach
Copy link
Owner

Unfortunately I don't have access to a Volkswagen anymore. If someone would be willing to test I would be happy.

DarkFox added a commit to DarkFox/volkswagen_we_connect_id that referenced this pull request Dec 20, 2024
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.

Send destinations to car
2 participants