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

Fixed weekday bug in vehicle.py #55

Closed
wants to merge 1 commit into from
Closed

Fixed weekday bug in vehicle.py #55

wants to merge 1 commit into from

Conversation

markblokpoel
Copy link

Line 628 contained incorrect reference to day, renamed it to weekdays. Should now return the correct schedule array.

I have not been able to test any of this, since I don't know how to incorporate the code edit in my home assistant instance.

Line 628 contained incorrect reference to day, renamed it to weekdays. Should now return the correct schedule array.
@Farfar
Copy link
Collaborator

Farfar commented May 16, 2022

I have reviewed the code but the "days" variable should be correct. The string sent to the API should be in the "yynnyny" format.

@Farfar Farfar closed this May 16, 2022
@markblokpoel
Copy link
Author

Ok, but then the logic is still faulty. It currently filters only the ‘y’ and hence returns incorrect strings. E.g. yynnyny become yyyy. My logs confirm this, see this issue.

@markblokpoel
Copy link
Author

I looked at the code again, didn't have time in a while, sorry. I believe that the check on line 627 in vehicle.py needs to be roved. If you look at the debug info in the post linked above you can see that this check basically filters all 'n's from the array. But as you correctly mentioned, the api expects a length 7 array. Do you want me to create a pull request for this?

@Farfar
Copy link
Collaborator

Farfar commented Jul 4, 2022

I will reopen this and take a look in the near future now when I have some spare time

@Farfar Farfar reopened this Jul 4, 2022
@Farfar
Copy link
Collaborator

Farfar commented Nov 23, 2022

Closing this PR since the change is incorporated since a while back.

@Farfar Farfar closed this Nov 23, 2022
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.

None yet

2 participants