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

Setting MQTT log topic to null produces error #6046

Closed
IetIesAai opened this issue Jul 16, 2024 · 3 comments · Fixed by esphome/esphome#7634 · May be fixed by esphome/esphome-docs#4412 or esphome/esphome#7723
Closed

Setting MQTT log topic to null produces error #6046

IetIesAai opened this issue Jul 16, 2024 · 3 comments · Fixed by esphome/esphome#7634 · May be fixed by esphome/esphome-docs#4412 or esphome/esphome#7723

Comments

@IetIesAai
Copy link

The problem

In documentation(https://esphome.io/components/mqtt) and #3047 it is mentioned setting

mqtt:
  log_topic: null

would disable logging via mqtt. However, when I do that and install the firmware wirelessly or attempt to retrieve the logs wirelessly, the error below appears. The upload itseld and the device do seem to work.

What might be relevant is I also disabled the api.

Which version of ESPHome has the issue?

2024.6.6

What type of installation are you using?

Docker

Which version of Home Assistant has the issue?

issue not relevant to home assistant

What platform are you using?

ESP32

Board

No response

Component causing the issue

mqtt

Example YAML snippet

mqtt:
  id: mqtt_broker
  broker: !secret mqtt_broker
  username: !secret mqtt_username
  password: !secret mqtt_password
  log_topic: null

Anything in the logs that might be useful for us?

Traceback (most recent call last):
  File "[...]/esphome/bin/esphome", line 8, in <module>
    sys.exit(main())
  File "[...]/esphome/lib/python3.10/site-packages/esphome/__main__.py", line 1079, in main
    return run_esphome(sys.argv)
  File "[...]/esphome/lib/python3.10/site-packages/esphome/__main__.py", line 1066, in run_esphome
    rc = POST_CONFIG_ACTIONS[args.command](args, config)
  File "[...]/esphome/lib/python3.10/site-packages/esphome/__main__.py", line 480, in command_logs
    return show_logs(config, args, port)
  File "[...]/esphome/lib/python3.10/site-packages/esphome/__main__.py", line 388, in show_logs
    return mqtt.show_logs(
  File "[...]/esphome/lib/python3.10/site-packages/esphome/mqtt.py", line 216, in show_logs
    topic = config[CONF_MQTT][CONF_LOG_TOPIC][CONF_TOPIC]
TypeError: 'NoneType' object is not subscriptable

Additional information

No response

@IetIesAai
Copy link
Author

also happens in 2024.7

@spuder
Copy link

spuder commented Oct 11, 2024

Do you get the same error with empty string? The following works for me. Notice that empty string is null, not string literal null

mqtt:
  # broker: broker.emqx.io # This server gives many TLS errors. 
  broker: 192.168.1.42
  port: 9993
  client_id: ${name}
  username: bblp
  password: 12345678
  log_topic:
  discover_ip: false
  discovery: false
  discovery_retain: false
  discovery_prefix:
  use_abbreviations: false
  topic_prefix: testtopic/1234

@solarkennedy
Copy link

solarkennedy commented Oct 11, 2024

And empty string gives

  not a valid value.
  log_topic: ''

Note that log_topic: null is not a string literal. It is a real null.
A string literal would look like 'null' (quotes to force the string)

The validation for this in the code is here:
https://github.com/esphome/esphome/blob/efe4c5e3bc496d9f1f4d152c28a27e505b9f3024/esphome/components/mqtt/__init__.py#L249

But this will fail because the topic is required:
https://github.com/esphome/esphome/blob/dev/esphome/components/mqtt/__init__.py#L75

And when it does get parsed it will become empty string anyway:
https://github.com/esphome/esphome/blob/efe4c5e3bc496d9f1f4d152c28a27e505b9f3024/esphome/config_validation.py#L1181

But this is just validation. The show_logs function isn't using this at all, and imo we are getting too far too late if it thinks it can use mqtt:
https://github.com/esphome/esphome/blob/dev/esphome/__main__.py#L369

The reason it thinks we should be using mqtt is because it is the only option available when we get to show_logs.
It doesn't pick api because you don't have api: in your config (like I don't) because the documentation says to remove it: https://esphome.io/components/mqtt.html

At this point I'm not sure what to do. Maybe it could print a friendlier error message, but fundamentally there are no show_logs options that will work. Too bad it can't use whatever method the web interface uses?

solarkennedy added a commit to solarkennedy/esphome that referenced this issue Oct 19, 2024
For esphome/issues#6046,
when MQTT logging is disabled (either via the `log_topic` or
`topic_prefix` set to null), the error message a user sees is python
exception.

This detects the configuration and prints a more human-readable error
message, reminding the user why MQTT logs are unavailable.
solarkennedy added a commit to solarkennedy/esphome-docs that referenced this issue Nov 5, 2024
Related to esphome/issues#6046,
it isn't necissary to remove the api component entirely.

In fact, it is kinda nice to keep it there for tailing logs if the
log_topic is `null`.

This clarifies that you can just set `reboot_timeout` and keep it
enabled.
solarkennedy added a commit to solarkennedy/esphome that referenced this issue Nov 5, 2024
This is a second pass at esphome/issues#6046

I have a situation where sometimes I have MQTT logging enabled, but
sometimes not depending on the device.

This PR removes the choice so that one doesn't have to remember that
MQTT logging is disabled, so that `esphome logs` goes right for the API
method (if available).
solarkennedy added a commit to solarkennedy/esphome-docs that referenced this issue Nov 5, 2024
Related to esphome/issues#6046,
it isn't necissary to remove the api component entirely.

In fact, it is kinda nice to keep it there for tailing logs if the
log_topic is `null`.

This clarifies that you can just set `reboot_timeout` and keep it
enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants