Refactor Touch class to extract Brush and Rub classes as their own touch features

[vberthiaume]

Fixes #22 and #23.

[vberthiaume]

I took this from the OG algorithm, but tbh I'm not entirely sure why we're ignoring values above 1?

[jcelerier]

I'll let Edu take the descision about the code semantics, here are just a few comments about the code. Thanks for this great patch !

[jcelerier]

Instead of this, maybe we should bass a reference to the float instead of a pointer. This way, the language itself prevents the null case as a reference must always point to something.

It would simply become

void tie(float& new_tie) {
    tied_data = &new_tie;
 }

as now we don't need a boolean to record if this worked :)

[vberthiaume]

ah of course, great one!

[jcelerier]

👍

[jcelerier]

I wonder if all these constructors are even needed, it should be equivalent to just not declare them as per rule-of-zero

[vberthiaume]

the issue is that if we want a constructor for the tied value (see the protected section), and I think we want one, then we also need at least a default one, and then might as well have all of them right. We could also rely on users explicitly calling tie to tie a value but that's a bit awkward

[vberthiaume]

I'm basically copying what was done (I think by you lol) here:

puara-gestures/include/puara/descriptors/jab.h

Line 39 in d708c6a

explicit Jab(double* tied)

[edumeneses]

nope, that was me 😅

[edumeneses]

But maybe simplifying might be good, there are indeed too many constructors

[jcelerier]

 const float sum = std::reduce(array + start, array + end); 

likely it will also give some performance boost as it may parallelize operations / use SIMD or funky stuff like that

[vberthiaume]

I'm personally reluctant to include anything that isn't necessary in embedded projects, so here <numeric>, but I guess it's all tradeoffs -- @edumeneses what do you think?

[jcelerier]

actually with concepts this can be done in a very built-in way:

#include <concepts>
float arrayAverage(const std::arithmetic auto* array, int start, int end) {

[jcelerier]

ideal code in an ideal world:

#include <concepts>
#include <numeric>
#include <print>

template<typename T>
concept arithmetic =  std::integral<T> || std::floating_point<T>;
float arrayAverage(const arithmetic auto* array, int start, int end)
// in a couple years, with C++ contracts (https://www.modernescpp.com/index.php/contracts-in-c26/)
//  pre(start >= 0)
//  pre(end >= start)
// until then it's asserts...
{
  return std::reduce(array + start, array + end) / float(end - start);
}

int main()
{
  {
    int foo[3]{0, 10, 20};
    std::println("{}", arrayAverage(foo, 0, 3));
  }

  {
    std::string foo[3]{"foo", "bar", "baz"};
    // build error since std::string is not arithmetic
    // std::println("{}", arrayAverage(foo, 0, 3));
  }
}

[jcelerier]

(std::arithmetic seems to be missing from stdlib)

[vberthiaume]

right... so in an ideal world concepts would be supported by the toolchain that platformIO uses, but for now they aren't 😅