-
Notifications
You must be signed in to change notification settings - Fork 639
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
Thingspeak: async client fixes #1806
Conversation
- remove memory management in async client callbacks - instead of global one, reference asyncclient from callback argument - transfer temporary data-buffer to the global scope, avoid accidental String copy - implement strnstr to search onData payload
I will try it; it seems a good solution. I would only add a handler for AsyncClient:onError, but I am not totally sure (is it necesary call to close in case of error? in case of dns error?). |
Error is any lwip-level error, including dns: Regarding string, you can check out WString.cpp in the core. Internally, it allocates on heap. Nice implementation detail is that it copies right-hand argument, replacing data but not buffer itself ( - DEBUG_MSG_P(PSTR("[THINGSPEAK] POST %s?%s\n"), THINGSPEAK_URL, _tspk_data.c_str());
+ DEBUG_MSG_P(PSTR("[THINGSPEAK] POST %s?%s [%p]\n"), THINGSPEAK_URL, _tspk_data.c_str(), _tspk_data.c_str());
Sorry, I missed the timestamping earlier but I think it is working ok right now. |
Ok, finally, starting testing under SonOff Pow R2 v2.0 and dev 1.3.16 (but from June 9... i din't see that there were new commits...), and using AsyncTCP 1.2.0 (https://github.com/me-no-dev/ESPAsyncTCP/compare/55cd520..7e9ed22 almost no difference with 1.1.3, only a potential null access when you close a connection already closed and other few fixes related to SSL). |
I have the same results with Sonoff Basic & temperature sensor.
Just curious, did you use my tree https://github.com/mcspr/espurna/tree/tspk/client-static or applied patches manually? (combined with asynctcp library version change). |
2 days running, no crash, no lost connection; all right.
I applied patches manually (I don't grasp git very well....). I took your last thinkspeak.ino ("commit promptly run flush" ad0e34a, included ) and manually edit prototypes.h and utils.h based in the other commits referenced in this pull request (declaration of strnlen and strnstr in prototypes.h and definition of strnstr in utils.ino); and yes, in platformio.ini I edited the dependences. I currently use I don't know if it is necessary git your tree completely, at least, with respect to thingspeak; anyway, as soon as I can, I will. |
You might want to check out: Or more manual method: Ofc, even more manual approach works too:
I think we need to add this to readme or something, to reduce the friction of pr testing... |
@JavierAder as discussed in #1752
Will try to leave it running with some sensor board.
There is another rough one, but greatly increasing firmware size:
dev...mcspr:tspk/client-fixes
+1280bytes vs +
48416bytes here(supporting newest Core versions requires some tweaks to the networking part. I hope that can be reused somewhere to justify code size increase)