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

MQTT improvements #422

Closed
wants to merge 1 commit into from
Closed

MQTT improvements #422

wants to merge 1 commit into from

Conversation

Radu-nic
Copy link

Hi guys - this is my first contribution ever on GutHub ;) so ... be gentle with me :D. I love this project!!! Great job!

I have used mqtt connection and i've found that something must be fix and other things should be nice to have so ...

MQTT improvements:

  • may change settings live, reading a persistent mqtt message with the same structure as the one saved in FS. The topic read is "ispindel/<>/settings". Every option saved into persistent local settings may be changed, but POLY and Sleep was those of maximum interest. After reading this new settings message this will be erase.
  • sent to mqtt broker an json report containing all readings on report topic This was useful for future writing to a log file or in my case to a database

MQTT fix:

  • fix on sending mqtt messages without user/password. If you do not set a username or password some mqtt brokers don't accept the message sending empty user&password
  • do not try send message if not connected after 3 retries.

  - may change settings reading a persistent mqtt message having the same structure as the one saved in FS. The topics read is "ispindel/*/settings"
  - sent to mqtt broker an json report containing all readings on report topic

MQTT fix:
  - fix on using mqtt broker without user/password
  - do not try send message if not connected after 3 retries
@stale
Copy link

stale bot commented Nov 9, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 9, 2020
Copy link
Owner

@universam1 universam1 left a comment

Choose a reason for hiding this comment

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

Nice work!
I’d like the remote configuration feature being optional which is disabled by default.
Reason it that it has a direct penalty on the runtime hence battery lifetime and that is why someone must be aware of that when enabling this option.
Sorry for the delay @Radu-nic there were conflicting PR arriving

@stale stale bot removed the wontfix label Nov 15, 2020
@Radu-nic
Copy link
Author

Radu-nic commented Dec 1, 2020

Hi Samuel.
I'm glad you had time to take a look at those changes and that you liked my work. I saw you fixed the issue sending mqtt message without connection, great! You do have right indeed about extra processing on "settings change" message. I will change the code to trigger this only when needed. Also I am trying to update my fork to master or cherry-pick your commits but ... no success. I will revert my branch and rewrite the changes.
So i suggest we abandon (delete) this pull request for the moment, don't know how to do it, i'm sure you do ;).
"I'll be back"

@stale
Copy link

stale bot commented Jan 30, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants