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 MQTT homeassistant sensors for air quality #13

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

glenrobertson
Copy link

Adds support for homeassistant sensor discovery and state reporting via MQTT

  • Add NVS variable for MQTT url (e.g. mqtt://user:[email protected])
  • Add MQTT lib which exposes a method for sending sensor data to a given MQTT url
  • Add task to call MQTT method when there is an MQTT url present

Notes:

  • I haven't yet tested the nvs variable setting - I tested with a hardcoded URL before adding this.
  • The NVS code changes are based on the weather path NVS code.

Relevant home assistant docs for MQTT based sensor discovery:

Happy thanksgiving 🦃

Update eink_display.c

wip2

Update mqtt.c

Update mqtt.c

Update mqtt.c

rename

move to task

Update mqtt.c

Add nvs property for MQTT url
bitclock-web/src/libs/gatt.ts Show resolved Hide resolved
bitclock-fw/main/tasks/eink_display.c Outdated Show resolved Hide resolved
bitclock-fw/main/libs/mqtt.c Outdated Show resolved Hide resolved
bitclock-fw/main/libs/mqtt.c Outdated Show resolved Hide resolved
bitclock-fw/main/libs/mqtt.c Outdated Show resolved Hide resolved
bitclock-fw/main/libs/mqtt.c Outdated Show resolved Hide resolved
bitclock-fw/main/tasks/mqtt.c Outdated Show resolved Hide resolved
bitclock-fw/main/tasks/mqtt.c Outdated Show resolved Hide resolved
bitclock-fw/main/tasks/mqtt.c Outdated Show resolved Hide resolved
bitclock-fw/main/libs/gatt.c Outdated Show resolved Hide resolved
@babldev
Copy link
Contributor

babldev commented Nov 29, 2024

Thanks! Did a quick first pass just looking at code. I'll have to do a closer look running on device and testing it out. Specifically memory usage, and backwards compatibility with the web app.

bitclock-fw/main/libs/mqtt.c Outdated Show resolved Hide resolved
bitclock-fw/main/tasks/mqtt.c Show resolved Hide resolved
@babldev
Copy link
Contributor

babldev commented Nov 30, 2024

Blocking things to test: (I can help with this too)

  • New web UI with old firmware (for people who haven't upgraded)
  • New web UI with new firmware
  • No change in behavior if MQTT server not set
  • No change in behavior if MQTT server removed (Bug: Does not show MQTT url on connect)
  • Stable for a few days with home assistant uploads

Non-blocking things to figure out:

  • Any significant temperature difference if MQTT server set (wifi does heat up device)
  • Packaging new release
  • Web UI update to indicate firmware release

@babldev
Copy link
Contributor

babldev commented Nov 30, 2024

Keeping some notes here. If you connect the new web UI to an old bitclock FW version, you will see:

  • Failed to read characteristic: 2ab48c15-934d-11ec-b90a-802748c27b9e: NotFoundError: No Characteristics matching UUID 2ab48c15-934d-11ec-b90a-802748c27b9e found in Service with UUID ec2f3aa0-4ed8-71bd-f147-b2bc37195232.

If you try to set the MQTT url:
Uncaught (in promise) NotFoundError: No Characteristics matching UUID 2ab48c15-934d-11ec-b90a-802748c27b9e found in Service with UUID ec2f3aa0-4ed8-71bd-f147-b2bc37195232.

Other changes seem to work. So I think this is fine but would be nice to branch instead of error if it detects an old firmware version. But I'd need to reveal that somehow.

@babldev
Copy link
Contributor

babldev commented Nov 30, 2024

If you update to new FW, and connect with new web UI, it initially doesn't work at all
Screenshot 2024-11-30 at 11 57 11 AM

https://github.com/goat-hill/bitclock/tree/master/bitclock-fw#bluetooth-gatt-attributes

Once you "Forget this device" and re-pair there are no errors. But something to think about communicating if people update.

@babldev
Copy link
Contributor

babldev commented Dec 1, 2024

Annoyingly I think we're running out of memory when parsing the weather API now with cJSON. I bet we were close to the limit and this MQTT stuff pushed us over :(

I (7958) weather_api: HTTP GET Status = 200, content_length = 12906
E (7968) weather_api: Error on JSON parse

I'll have to poke around for ways to reduce memory usage.

@babldev
Copy link
Contributor

babldev commented Dec 1, 2024

Had to throw a ton of changes in here, mostly because the weather API is extremely expensive to the heap right now. It uses about 30kB via the cJSON library to parse the 17kB giant response from api.weather.gov.

I made a bunch of other tweaks in my testing that leaves about 3kB worst case of extra headroom. Not great because a small weather.api.gov change could push things over the limit.

Long term fix would be to use a streaming parser of JSON instead of a one-shot one. Those are harder to implement but way less memory intensive. https://components.espressif.com/components/espressif/jsmn instead of cJSON

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.

2 participants