Closed ZionFox closed 6 months ago
Hi @ZionFox,
thank you for your analysis. Since email addresses can contain special characters like the plus sign (+) which has a special meaning in MQTT topics we had to introduce a method that removes those special characters.
Without this method the gateway crashes right on the start when it tries to subscribe to some topics.
Of course we have to do the same special character removal when we publish something. Unfortunately, this is still missing.
Sorry for that. This is hard to test without a SAIC user name that contains a special character.
I'm glad I can be the edge case for testing, then. I figured it was, fortunately the debug level of logging correctly showed the string being used for the requests, so I could identify the difference in character difference.
I understand MQTT requires string santiation, it just seems it was implemented when subscribing to topics, and overlooked when publishing. I'm looking forward to the patch, but if the PR has that patch I'll test it now.
Can confirm that PR https://github.com/SAIC-iSmart-API/saic-python-mqtt-gateway/pull/214 does indeed resolve this issue. String sanitation that was omitted was not playing nicely when publishing or fetching data to MQTT.
I found the fix to this, it's at the bottom of this report.
I've attempted to set up as instructed in the guide, but am hitting a wall with the warning
TRYING TO WRITE TO CLOSED SOCKET
being thrown for every topic update, except for thesaic/_internal
one.Environment:
Debian 12 bookworm running Docker 25 Home assistant 2024.4.3 running in Docker Mosquitto 2.0.18 MQTT broker running in Docker
I've tried both the 5.15 release, and the 0.6.0rc25 pre release versions of the saic-mqtt-gateway, via the docker image offered. I've tried pulling the
:latest
image, and also building the image from the Dockerfile.Logs:
This continues until it hits an exception, and dumps the following stacktrace:
Then it continues.
Key notes:
I use a special character in my email address for the SAIC account. The
+
is appearing differently in the logs. At the start when publishing theSEND SUB
messages, it's replaced with an underscore, yet for all subsequent messages, it displays correctly in the logs.However, the
saic/_internal
topic is being published and regularly updated to MQTT, which can be verified by an explorer. It does seem to understand details about my car, it just doesn't seem to be making this auto-discoverable in Home Assistant, possibly due to the symbol in my email address?Fix
I dove deeper into the code for this, and done some debugging.
It turns out that when doing the MQTT subscriptions, the input string is being cleaned up with the method
remove_special_mqtt_characters()
located inmqtt_publisher.py
line 60, and the result of that cleaned up string is being placed in a variable that is then used when subscribing to topics. This seems to be an "initialise" of four topics, relating to the vehicles VIN.However, when looking to publish this data in
mqtt_gateway.py
line 418 (of the 5.15 codebase, it's in a similar location in the 6.0 rc) theaccount_prefix
variable is just taking the siac_user object key as is, and not doing any clean-up of it. I updated my own internal version of the 6.0 code base to a crude fix, and it now works, publishing the data to the broker.This removed all of the
TRYING TO WRITE TO CLOSED SOCKETS
errors, an the data is now properly formed in the MQTT broker. I highly suggest performing this fix (and ensuring other character escaping is done in key places) before releasing 6.0.