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

WIP: make sensors web configurable #54

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

mpolitze
Copy link

Thank you for the awesome work! I had the issue that I did not want to recompile the firmware when installing an ESP. To solve that I moved configuring sensors to the web interface. I also added an option to publish JSON formatted MQTT payloads which is required by thingsboard and added a small /status page. I would appreciate if you accept this pull request.

Note: The changes ran quite smoothly for several weeks. I just made final adoptions to merge your latest changes from v3.0.0 and updated my ESP. The last hour seemed good. I will give feedback in a couple of days if everything is still running as expected.

@mruettgers
Copy link
Owner

Thank you! This would be a great addition to the project. I'll look forward to your feedback.

I found one thing, that is likeley to make troubles and I'll comment it within the code.

{
const char* willTopic = lastWillTopic.c_str();
const char* buf = new_json_wrap(willTopic, MQTT_LWT_PAYLOAD_OFFLINE);
client.setWill(willTopic, MQTT_LWT_QOS, MQTT_LWT_RETAIN, buf);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While working on the latest changes (including the replacement of the MQTT library), I noticed that the MQTT client won't reconnect, if the pointer to the last will topic has become invalid. That's why I stored the last will topic in a previously defined variable in the scope of the class. I think we should do it this way for the JSON wrapped topic, too.

@mpolitze
Copy link
Author

thanks for the review! I added a couple of lines to use the stored lastWillTopic and to keep the lastWillPayload in memory in the json case.

@mruettgers mruettgers mentioned this pull request Nov 17, 2022
@mpolitze
Copy link
Author

The sensor now ran with the merged changes for the last couple of days. MQTT survived router reboots and internet connection losses in between. I would greatly appreciate if this could be merged.

@mruettgers
Copy link
Owner

I am sure that I will merge the PR. But before that I would like to test it myself and we still need to update the manual because the configuration is not preserved during the update and the configuration via web interface still needs to be explained in the readme.

Since I have very little time right now, I don't know when I will get to it.

@mruettgers
Copy link
Owner

I haven't forgotten the PR, I'm just busy with my bachelor thesis at the moment and won't have time to work on SMLReader again until March.

@GreyPenguin
Copy link

Good luck with the thesis

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.

3 participants