Mqtt optimisations and TZ fixes#1205
Conversation
Some Flak8 fixes, some adjustments to logging levels, ie warnings and errors
WalkthroughRefactors the MQTT publisher: replaces explicit comparisons with truthiness checks, adjusts logging levels/messages, returns False when disconnected, localizes naive timestamps with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as Publisher
participant Cfg as Config
participant TZ as conf.tz
participant MQTT as MQTT Client
App->>Cfg: check_config()
alt config truthy
App->>MQTT: check connected (truthiness)
alt connected
App->>App: ensure message is dict/string
App->>App: convert non-strings → JSON
App->>TZ: localize naive timestamp via tz.localize
App->>MQTT: publish(topic, payload, qos, retain)
MQTT-->>App: publish result
App-->>App: log (debug/minimal)
else not connected
App-->>App: log not-connected (minimal)
App-->>App: return False
end
else config missing/false
App-->>App: log config issue
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
front/plugins/_publisher_mqtt/mqtt.py(15 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
front/plugins/*/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
front/plugins/*/*.py: Plugin scripts must log only via mylog(), never using print()
Collect results with Plugin_Objects.add_object(...) during processing and call plugin_objects.write_result_file() exactly once at the end
Write results exclusively to /app/log/plugins/last_result..log using Plugin_Objects; do not write ad‑hoc result files
When creating/updating devices, define database_column_definitions and ensure watched fields map to notifications
Validate and normalize MAC addresses (use helper normalize_mac) before any DB write or object emission
Files:
front/plugins/_publisher_mqtt/mqtt.py
{server/**/*.py,front/plugins/*/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{server/**/*.py,front/plugins/*/*.py}: Never hardcode ports or secrets; define settings once (core ccd() or plugin manifest) and read via get_setting_value() everywhere
Use logger.mylog(level, [message]) with levels none/minimal/verbose/debug/trace; keep verbose output concise unless debugging
Reuse helpers in helper.py (timeNowTZ, normalize_mac, sanitizers) and prefer server/db/db_helper.py over raw SQL in new paths
Files:
front/plugins/_publisher_mqtt/mqtt.py
🧬 Code graph analysis (1)
front/plugins/_publisher_mqtt/mqtt.py (4)
server/plugin_utils.py (1)
getPluginObject(246-276)server/logger.py (3)
mylog(86-89)Logger(55-93)timeNowTZ(16-20)server/helper.py (5)
timeNowTZ(35-45)get_setting_value(358-411)bytes_to_string(603-607)sanitize_string(642-646)normalize_string(651-657)server/database.py (1)
get_device_stats(256-272)
🪛 Ruff (0.13.1)
front/plugins/_publisher_mqtt/mqtt.py
349-349: Unused function argument: userdata
(ARG001)
374-374: Unused function argument: mqtt_client
(ARG001)
374-374: Unused function argument: userdata
(ARG001)
374-374: Unused function argument: flags
(ARG001)
374-374: Unused function argument: properties
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker_dev
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
front/plugins/_publisher_mqtt/mqtt.py(14 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
front/plugins/*/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
front/plugins/*/*.py: Plugin scripts must log only via mylog(), never using print()
Collect results with Plugin_Objects.add_object(...) during processing and call plugin_objects.write_result_file() exactly once at the end
Write results exclusively to /app/log/plugins/last_result..log using Plugin_Objects; do not write ad‑hoc result files
When creating/updating devices, define database_column_definitions and ensure watched fields map to notifications
Validate and normalize MAC addresses (use helper normalize_mac) before any DB write or object emission
Files:
front/plugins/_publisher_mqtt/mqtt.py
{server/**/*.py,front/plugins/*/*.py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
{server/**/*.py,front/plugins/*/*.py}: Never hardcode ports or secrets; define settings once (core ccd() or plugin manifest) and read via get_setting_value() everywhere
Use logger.mylog(level, [message]) with levels none/minimal/verbose/debug/trace; keep verbose output concise unless debugging
Reuse helpers in helper.py (timeNowTZ, normalize_mac, sanitizers) and prefer server/db/db_helper.py over raw SQL in new paths
Files:
front/plugins/_publisher_mqtt/mqtt.py
🧬 Code graph analysis (1)
front/plugins/_publisher_mqtt/mqtt.py (5)
server/plugin_utils.py (1)
getPluginObject(246-276)front/plugins/plugin_helper.py (2)
Plugin_Objects(245-304)write_result_file(288-291)server/logger.py (3)
mylog(86-89)Logger(55-93)timeNowTZ(16-20)server/helper.py (5)
timeNowTZ(35-45)get_setting_value(358-411)bytes_to_string(603-607)sanitize_string(642-646)normalize_string(651-657)server/database.py (2)
get_device_stats(256-272)open(35-77)
🪛 Ruff (0.13.1)
front/plugins/_publisher_mqtt/mqtt.py
350-350: Unused function argument: userdata
(ARG001)
375-375: Unused function argument: mqtt_client
(ARG001)
375-375: Unused function argument: userdata
(ARG001)
375-375: Unused function argument: flags
(ARG001)
375-375: Unused function argument: properties
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: docker_dev
🔇 Additional comments (1)
front/plugins/_publisher_mqtt/mqtt.py (1)
416-420: Don't mark the broker connection as established untilon_connectfiresThis is the same problem flagged in the earlier review: setting
mqtt_connected_to_broker = Trueimmediately afterconnect()means a failed handshake (wrong credentials/broker down) still flips the global to True.publish_mqttthen skips its guard, spins onmqtt_client.publishagainst a disconnected client, and never recovers cleanly. Leton_connectbe the sole place that sets the flag to True once the broker actually acknowledges the session.- err_code = mqtt_client.connect(host=get_setting_value('MQTT_BROKER'), port=get_setting_value('MQTT_PORT')) - if (err_code == mqtt.MQTT_ERR_SUCCESS): - # We (prematurely) set the connection state to connected - # the callback may be delayed - mqtt_connected_to_broker = True + err_code = mqtt_client.connect(host=get_setting_value('MQTT_BROKER'), port=get_setting_value('MQTT_PORT')) + if err_code != mqtt.MQTT_ERR_SUCCESS: + mylog('minimal', [f"[{pluginName}] ❌ MQTT connect failed with rc={err_code}"]) + mqtt_connected_to_broker = False + return None
| if not isinstance(message, str): | ||
| message = json.dumps(message).replace("'",'"') | ||
| message = json.dumps(message).replace("'", '"') | ||
|
|
There was a problem hiding this comment.
JSON payloads are being corrupted before publishing
json.dumps already produces valid JSON; replacing every ' with " will mangle legitimate apostrophes inside string values (e.g., O'Hara) into invalid JSON and change the data Home Assistant receives. Please drop the .replace("'", '"') so we send the canonical dump untouched.
- message = json.dumps(message).replace("'", '"')
+ message = json.dumps(message)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not isinstance(message, str): | |
| message = json.dumps(message).replace("'",'"') | |
| message = json.dumps(message).replace("'", '"') | |
| if not isinstance(message, str): | |
| message = json.dumps(message) |
🤖 Prompt for AI Agents
In front/plugins/_publisher_mqtt/mqtt.py around lines 270 to 272, the code uses
json.dumps(message).replace("'", '"') which corrupts valid JSON by replacing
apostrophes; remove the .replace("'", '"') and simply assign message =
json.dumps(message) when message is not a string so the canonical JSON produced
by json.dumps is sent unchanged.
There was a problem hiding this comment.
It's working, so not intending to change.
There was a problem hiding this comment.
@ingoratsdorf, understood! If it's working in your environment, no need to change it. Thanks for the clarification.
(\ /)
( ._. )
o_(" ")(" ")
|
Thanks for the PR @ingoratsdorf - LMK if good to merge |
|
Working from my end. Good to merge. :-) |
2 participants
A bit of cleanup and optimisation work.
Fixes bug #1204
Deleted unused imports
Added source code documentation
Summary by CodeRabbit
Bug Fixes
Refactor