Simplification of implementation for new position sensors and current senses: generic sensor classes

[askuric]

A new approach for easier support of different sensors in the SimpleFOC.

I've created generic implementations for the position sensor (GenericSensor) and current sense (GenericCurrentSense).

Generic position sensor

GenericSensor requires the user to provide the sensor.readCallback function pointer which returns the float angle reading in between 0 and 2PI. Everything else is handled by the GenericSensor class. Additionally the user can provide the initialisation routine also sensor.initCallback which if provided is called in the sensor.init(). There are two ways of providing the functions to the GenericSensor

float readMySensor(){
 // read my sensor
 // return the angle value in radians in between 0 and 2PI
 return ...;
}
void initMySensor(){
  // do the init
}


// Provide the callbacks in the constructor
GenericSensor sensor = GenericSensor(readMySensor, initMySensor);

or

// empty constructor 
GenericSensor sensor = GenericSensor();

void loop(){
...
  // assign the callbacks directly
  sensor.readCallback = readMySensor;
  sensor.initCallback = initMySensor;
  sensor.init();
....
}

Generic current sense

GenericCurrentSense requires the user to provide the current_sense.readCallback function pointer which returns the PhaseCurrent_s stucture with current readings in Ampers. Everything else is handled by the GenericCurrentSense class. Additionally the user can provide the initialisation routine also current_sense.initCallback which if provided is called in the current_sense.init(). There are two ways of providing the functions to the GenericCurrentSense

float readMyCurrentSense(){
 // read my sensor
 // return the value in Ampers
PhaseCurrent_s c;
c.a = ..
c.b = ..
c.c = ...
 return c;
}
void initMyCurrentSense(){
  // do the init
}


// Provide the callbacks in the constructor
GenericCurrentSense current_sense = GenericCurrentSense(readMyCurrentSense, initMyCurrentSense);

or

// empty constructor 
GenericCurrentSense current_sense = GenericCurrentSense();

void loop(){
...
  // assign the callbacks directly
  current_sense.readCallback = readMyCurrentSense;
  current_sense.initCallback = initMyCurrentSense;
  current_sense.init();
....
}

I hope this will speed up and simplify the process of supporting the new sensors!

[runger1101001]

I like the generic sensor, this could be simpler for users to understand than the API based on subclassing the Sensor class...

I still have to think about the generic current sense. I guess the same applies here, but it is more tricky because of all the issues we discussed in the last call...

I've been working on that as we discussed, but I'm not yet ready to commit anything.

[askuric]

Yeah, you are right. This add-on is going to simplify both in my opinion. And also we will be able to support low-side current sensing easier here I think. Because the users can find the code that works for them then just plug it to the simplefoc using this.

Since it follows the same convention as the Inline and LowSide for the moment I'd merge this as is. To provide the users the way to test and try out. Maybe also leave the current sense as is for v2.2.1. Then do a proper restructure in v2.3.0 for example.
What do you think abou that?

[runger1101001]

Regarding the release schedule, I agree with that, because there are other fixes on the dev branch that people would want in the meantime.

Regarding plugging the current sense, I thought about it a bit, and I think this would remain a "pro user" thing. This is because even with the simplified callback based API for the current-sense the user will still have to know which timers the SimpleFOC configured in the driver in order to set up the DMA/events/interrupts correctly.

It is true that this might help users in the meantime, since they will maybe "know" the timers used for their board and pin configuration, and could just hard-code it. But at the moment there is no API to query the timers from the driver so they could not make a general solution.

On the other hand, this means it would lead to a bunch of timer-specific, pin-specific, board-specific code, which might really confuse things when people start sharing it in the forum...

So I'm not sure on the current sense.

[askuric]

Ok, so current sense will, even with this change remain pro user thing. Exactly, they do need to know how sensors work in order to use these generic sensors . User will need to either figure it out or use someone else's code to do it. But the good thing here is that the someone else's code does not have to be simplefoc related.
Maybe someone wants to add an external ADC and use with some custom communication or just found a code that works and they would like to integrate with the simplefoc.
Since there is an universe of possible combintations of features that we would (will) have to support to make the true current sense abstraction for everyone, this is a simple way for the community to contribute without needing to go to the depths of our API.

Now, is it a good way, maybe not. But at the moment it is the simplest one.

And in terms of the confusion abut the timers and low level api in the community, I think this is actually good, because some good solutions will come out and that will maybe help us make right choices in future about the most natural way to support hardware.

So I am still pretty much confident that this can be merged, now if you really think that this is a bad idea, then I'll split the code and we will talk about this thing some other time. :D

[runger1101001]

I say go for it! :-)