Use generic typechecks, OTA add followRedirects

[Niek]

This PR changes the following:

• Add CREATE_CHECK(class, function, params...) macro to generate typechecks
• Change typecheck in HeapStats to use above macro
• Add typecheck in OTA_CLIENT_HTTPUPDATE module instead of OTA_CLIENT_HTTPUPDATE_2_3_0_COMPATIBLE macro
• Add typecheck in OTA_CLIENT_HTTPUPDATE module for followRedirects(), enable if available

[mcspr]

Based on gitter discussion, I tried adding this as a struct member with an earlier example

static constexpr std::integral_constant<bool, value> value_type{};

Where value is bool above, thus value_type is mapped to false or true type. But I can't build it locally with gcc-7 resulting in undefined reference errors for value_type. esp's gcc-4.8.2 builds this (...idk if it runs properly though), not sure what the problem is with this kind of use of static variable. The intent is the same as here - use two overloaded templates with value_type as the first arg

Maybe something like is_detected is better for the reader?
My main concern is having class name and class instance in a different places. Since this is now generic macro, maybe check variable should be replaced with typed_check<decltype(OBJ)> / hidden under another template typed_check<T> like in the example above?

[Niek]

Yes, there are many nicer solutions (std::experimental::is_detected being the best looking one), but those do not compile on "older" compilers. I couldn't even get that to work on GCC < 9. But I agree that the namespace clutters quite a bit. Can you add your proposal as a commit to this PR?

[mcspr]

Will do.

gcc headers are a bit tricky to navigate to c/p everything needed, but I have tried implementation from clang and small example from sol2 issue tracker, which both build just fine:
https://github.com/llvm-mirror/libcxx/blob/master/include/experimental/type_traits
ThePhD/sol2#784 (comment)

Current implementation was made up based on something in the gcc sources (i think i just grep'ed for true_type) and I wonder what is the reason for using value_t = {true,false}_type; instead of inheritance like sol2 example and the following proposal paper. I guess I need to read this more closely.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4502.pdf

[mcspr]

Adding Serial.println(...) into the appropriate types and trying to build with 2.3.0, 2.5.0 and 2.5.2 shows appropriately detected function versions simply by using strings firmware.bin:

# 2.3.0
false_type getheapstats
false_type follow redirects
false_type wificlient arg
# 2.5.0
true_type getheapstats
false_type follow redirects
true_type wificlient arg
# 2.5.2
true_type getheapstats
true_type follow redirects
true_type wificlient arg

[mcspr]

btw shouln't this be back where it was initially? check for has_WiFIClient_argument_t there and supply it when true.
i don't remember the specifics (search is not helping...), but there were also a couple of issues with httpclient & wificlient destructor ordering. but, this may not be relevant if httpclient can handle wificlient missing and update() does not keep lingering connection / expects more actions.

edit: and even more so, isn't make_unique struct + object destroyed right after .get()?

[Niek]

I moved it inside the function because that means one place less to check the update() signature: https://github.com/xoseperez/espurna/pull/1974/files#diff-5727c97832f80d3ac6aedd50dbe75fd6L81-L90 - but yes, not sure if this is the best approach.

[mcspr]

Note the second comment, WiFiClient is destroyed right after get().

[Niek]

Hmm, that was not the case previously when it was called in the _otaClientFromHttp() function? I tested OTA with these changes and I didn't run into any issues so far.

[mcspr]

Not the case, unique_ptr lived while parent function was active.
idk how that works exactly... effectively, wificlient is no longer allocated and anything can claim that memory segment through new / malloc

[mcspr]

Right. It could still check for arg availability though. Only cost is +2 lines, everything is known at compile time.

[Niek]

So split _otaClientFromHttp() method and call the appropriate one from _otaClientFrom() with the type check? Or you mean a different way?

[mcspr]

I meant right in the fromhttp, optionally creating wificlient. unique_ptr structure could be initialized with nullptr, .get() will return that null pointer.

[Niek]

Sorry but I'm not sure I follow - feel free to commit what you have in mind :)

[mcspr]

https://godbolt.org/z/yu9fxq <-
Maybe I misunderstood the question. make_unique<T>() == unique_ptr<T>(new T()), so we just hide make_unique under the check