Feat pwm spi #92
Feat pwm spi #92
Conversation
use of EVSYS to trigger ADC conversion at each TCC OVF removed DMA code as I couldn't figure out how to combine both Problem1: Currently I only get straight 1.71V +/- 0.01 no matter what happens with the motor /control loop. Something is off with the timing Problem2: all OVF signal triggers the ADC, which is configured in INPUTSCAN mode and iterate over the 3 channel at each event trigger. while events come from different channels, they all sink into the same USER channel and there is now way of correctly mapping e.g. TCC0 to ADC pinA
replaced a few const char* with PSTR()
new _SAMD21_EVSYS_ define
|
Awesome! Thanks so much for all this work!! I'd love to just merge it, but I fear there are a couple of issues:
|
|
Another question, looking at it - will it support more than one motor? It seems the current sensing code is using a variable called instance - maybe I don't understand the API fully yet... |
|
Hello! Thanks for the review. I tried to touch the driver side as little as possible. I agree it is not perfect as it is. I think it could work with TC units. For the sensor API, I just created a new abstract base class which contains most of the common logic. Unfortunately, Arduino's SPIClass is pretty much sealed up so I had to create a new one. Maybe we could use templates and duck typing to create abstraction. The only real reason for the "instance" thing is to please the current sensing API, which would not support more than one motro either. To support a second motor, you would need a second ADC (samd, same), though. I repeat, the current code uses the ADC in ** input scan** mode. When it receives an event, it starts the scan process, and samples pins A,, B, C, then stop. this is all hardware. What we "could* do is scan more than 3 pins (e.g. 6) on every signal, but it would becomes messy. better use interrupts and suffer. |
|
so @runger1101001 For example, to use sercom5, you have to call Also, my class is nicely telling you if you're using a valid pin/pad assignement. we could make the SPI type a template parameter (which defaults to SPIClass) and reproduce SPIClass API. But if that's what you really prefer, I'll move it to drivers |
|
Hey @maxlem, First of all thanks for the great work. The depth you have gone into is impressive. 😄 Now, in terms of low side current sensing I cannot really test it at the moment, I do not have any samd21 chips with me. As soon as we can test it I would be in to merge it. Or even merge it before but I'll have to document everything properly so that the people know it has not been properly tested. Now, in terms of SPI sensor, I'd move this to the drivers repository. It is far too much for this library. It is really cool what you have done, but simplefoc is really not the "advanced sensor driver" but the FOC driver. I know we have mixed a lot of stuff already in this repository but we are always trying to support as little as possible hardware specific code inside of the simplefoc. Now in terms of api changes.
At this point we are restructuring the library and we are going to allow for the communication in between driver code and current sensing code in a form of shared configuration structures or classes or something like that. We have still not decides 100%. |
|
Although, i have to say that i really like how you've added the hardware_specific folder to the senors also. Do you think it's a good idea to have all hardware specific code in the same place, or you think it would be better to leave the stuff as is. As is maybe makes more sense for sensors. |
|
Hey - I'll make myself available and find some time over the course of next week to integrate @maxlem 's code. If it is ok with everyone I would put all the sensor stuff into the drivers repository for now, and concentrate on getting the current sensing into the main repo. I think we need to be careful not to introduce to many new changes in one release, or we'll get in a muddle, or have lots effort testing - or have lots of bugs. And also I would focus on changes that "most users" would benefit from for the main repo (for now). And although I quite like it after writing all that code for it, SAMD21 isn't the architecture most used with SimpleFOC. We made the drivers repo to have a place for all the often hardware specific code that is clearly useful and connected to SimpleFOC, but probably not used by most of the users. It's also so we can have different development cycles and levels of testing on these two code bases. All this is of course 100% your call, Antun, but that's how I would see it. And regarding your question, Antun, my opinion is that leaving the sensors as is for now is better. We can always come back and revisit that question next release... |
|
All good points! Personally, I don't like this _apiFunction() with global variables hidden in cpp files. We should use OO instead. If you fear for virtual table lookups, we can always use generic programming (templates) instead I will move the SPI stuff away from this MR, I don't really mind. Still, I think my abstract MagneticSensor class is right on the money (except maybe the name). I think this class should be the base class for any sensor that has an absolute reading in some integer form. |
one widespread change: _readADCVoltagesLowSide(a,b,c)
Justification: only used in LowsideCurrentSense, we're always reading in pairs(triple) and it enables async (interrupt, dma) and concurrent (multiple ADC) implementations
the rest of potentially conflicting changes are guarded by the SAMD21_ASYNC tag
this MR brings in a MAJOR effort from my part to enable async ADC and SPI readings with the samd21