-
Notifications
You must be signed in to change notification settings - Fork 127
added wildcard handling for handle_external_mqtt_message #1787
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
base: main
Are you sure you want to change the base?
Conversation
|
From memory it is deliberate design decision to not support wildcards on the internal EVerest api topics. |
|
Oh ok, i'll avoid using them then. |
Pietfried
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ThatsLucas , thanks for the contribution. It is ok to add wildcard support for external mqtt.
The current approach modifies execute_handlers_from_vector which affects both var_handlers and external mqtt messages but we would like to keep var_handlers separate.
handle_error_message is already considering wildcard topics, so we can make use of the same approach.
My suggestion is to define execute_handlers_from_vector_with_wildcards as a member function and use it within handle_error_message and handle_external_mqtt_message like:
void MessageHandler::handle_external_mqtt_message(const std::string& topic, const json& data) {
execute_handlers_from_vector_with_wildcards(external_var_handlers, topic,
[&](const auto& handler) { (*handler->handler)(topic, data); });
}
void MessageHandler::handle_error_message(const std::string& topic, const json& data) {
execute_handlers_from_vector_with_wildcards(error_handlers, topic,
[&](const auto& handler) { (*handler->handler)(topic, data); });
}Signed-off-by: ThatsLucas <lucasmailpro9@gmail.com>
d906300 to
5024893
Compare
…n and now use vector for errror_handler Signed-off-by: ThatsLucas <lucasmailpro9@gmail.com>
5024893 to
cd61b19
Compare
|
Hey @Pietfried thanks for the suggestion ! I made the requested change, looks good for me. Is this the intended behavior, or was it meant to replace the handler each time? |
Pietfried
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Describe your changes
This PR fixes the issue reported in #1712 where the MQTT message handler was not working with topics containing wildcards.
I used the already existing function 'check_topic_matches'
Issue ticket number and link
Checklist before requesting a review