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

Event callback system #149

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

Event callback system #149

wants to merge 4 commits into from

Conversation

perigoso
Copy link
Contributor

@perigoso perigoso commented Dec 14, 2021

this is WIP.

Adds an "event" callback system.

There is now a single user callback, this is called from mqtt_sync whenever theres an event relevant for the user, like a publish acknowledged or a publish received, the reconnect callback is now also handled by this.

I think maybe we want to add some sort of flag system, to enable/disable what callbacks we want, for now, everything is enabled as long as the callback is not null.

These changes need a version bump, they break compatibility with the current implementation, i thought baout keeping compatibility, but i think this approach is better and it required the breakage.

note: built on top of #148

TODO:

  • modify examples to use new api
  • add enable/disable to callbacks

closes #98

@perigoso
Copy link
Contributor Author

These changes have been under test in our application using an ESP w TLS, and so far everything looks good

@LiamBindle
Copy link
Owner

Thanks for this @perigoso. I'm going to make a branch called feature/callbacks and merge this there. Future PRs and development work to the callback system can go there. Once we're certain it's working correctly we can merge it into main.

@perigoso
Copy link
Contributor Author

perigoso commented Mar 8, 2022

sounds good 👍

@perigoso
Copy link
Contributor Author

perigoso commented Mar 8, 2022

commit e2c1ecf should be cherry picked and merged on master though, as it's a bugfix

@fariouche
Copy link

hello!
So, is it available somewhere now?

@perigoso
Copy link
Contributor Author

Its not merged into the main repository, but it is available in the fork
aj-tec:callbacks

Signed-off-by: Rafael Silva <[email protected]>
Signed-off-by: Rafael Silva <[email protected]>
@perigoso
Copy link
Contributor Author

commit e2c1ecf should be cherry picked and merged on master though, as it's a bugfix

cherry picked into new PR #167

@inobelar
Copy link

inobelar commented Oct 7, 2022

Wow, it's amazing @perigoso, @LiamBindle ! I'm experimenting with mqtt-over-websockets and webassembly (via emscripten), and found that this library perfectly suitable for it (mqtt client running in browser, written in C/C++)! It tiny, readable/hackable, easy to integrate - just perfect to use, in comparison to such (huge) alternatives like Qt + qmqtt or Eclipse::Paho (which, without huge modification, still not work for me).

Due to event-driven (callback-driven) design of websockets, I'm also incredibly interested in callback-driven design of this library. I'm found this PR, trying to figure such simple things like "how to check is client connected", or "how to subscribe/publish after successful connection" (well, as I see, currently (in master branch) that's only in internal state, without notification to outer space, and this PR fixes it with beautiful generalization).

Callbacks allow us to write C++ wrapper, which behave beautifully the same as Qt (I'm C++ programmer which previously uses Qt on my work). Pseudocode:

mqtt::Client client( /*send_buff_size*/ 2028, /*recv_buff_size*/ 2048);
client.onConnect([&client] {
    client.subscribe("/test/topic");
});
client.onDisconnect([] { /*noop or log here */ });
client.onMessage([](const mqtt::Message& msg) {
    /* process msg.topic() / msg.payload() here */
});
client.onError([](mqtt::MQTTErrors error) { /* log with: mqtt_error_str(error); */ } );

client.setHostname("ws:https://127.0.0.1");
client.setPort(1884); 
client.connectToHost();

while(true) { /* Explicit way, may be hidden in internal thread */
    client.sync();
}

So I'm personally really looking forward to when it will be merged into master (since, as I see, only readme & examples currently not modified to new design) :)

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.

Generalise calback system?
4 participants