panda-official / TimeSwipe

PANDA Timeswipe driver and firmware
GNU General Public License v3.0
3 stars 5 forks source link

add analog out support #47

Closed mkh-welsch closed 4 years ago

mkh-welsch commented 4 years ago

Output of a defined square wave. amplitude, frequency, shift_amount, numbers.

Definition of Done

LeilaPiri commented 4 years ago
Amplitude

Set a default for duty_cycle (T_ons(highs) /T_total)= 0,5 (User should be able to reset new value) Function should look like this for 2 analog outputs:

  1. output1_pwm (high, low, frequency, repeats, duty_cycle)
  2. output2_pwm (high, low, frequency, repeats, duty_cycle)

https://www.arduino.cc/en/tutorial/PWM https://www.exp-tech.de/blog/arduino-tutorial-pulsweitenmodulation-pwm

IngoKaiser commented 4 years ago

For toggling on/off for Analog out on 3/4 we use DACsw.

We should have two ways setting AOUT3/AOUT4: First is just static High/Low, other is PWM just like @LeilaPiri has described above.

@iluxa We need to be able to set repeats of pwm e.g. for stepper engine. What do you think about dealing multiple commands from user? Restarting every time when new command arrives? Chaining?

Or even having AOUT Setting (static, pwm) and start/stop by toggeling? But how do defined number of repeats then?

iluxa commented 4 years ago

If output1_pwm and output2_pwm going to be implemented as API driver function it is possible to limit their usage before Start call, user can exex sequence like Init -> output1_pwm -> Start -> Stop -> output2_pwm -> Start

or is it going to be implemented as interactive functions during Start is active?

IngoKaiser commented 4 years ago

I guess it would be nice having it independent from Start would be good.

One general question: Can methods of TimeSwipe been called from main or also within callbacks?

iluxa commented 4 years ago

Can methods of TimeSwipe been called from main or also within callbacks?

Not all methods can be called from callback (like init methods), for examle Stop can be called from any callback.

iluxa commented 4 years ago

Here 3 new API call suggested for PWM generator:

    /**
     * \brief Start PWM generator
     * Method can be called in any time.
     * PWM generator works only during reading sensor loop - between @ref Start and @ref Stop calls
     *
     * @param num - output number - possible values are 1 or 2
     * @param frequency - periods per second - frequency > 0
     * @param high - PWM signal high value - possible values are 0..4096. high >= low
     * @param low - PWM signal low value - possible values are 0..4096. low <= high
     * @param repeats - number of periods to repeat. PWM generator will work (repeats/frequency) seconds
     *                  after repeats number get exhausted PWM goes to Stop state and StartPWM can be called again
     *                  0 is for unlimited repeats
     * @param duty_cycle - part of PWM period when signal is in high state. 0 <= duty_cycle <= 1
     *
     * @return false if at least one wrong parameter given or generator already in start state
     */
    bool StartPWM(int num, int frequency, int high = 4096, int low = 0, int repeats = 0, float duty_cycle = 0.5);

    /**
     * \brief Stop PWM generator
     * Method can be called in any time.
     * PWM generator works only during reading sensor loop - between @ref Start and @ref Stop calls
     *
     * @param numb - output number - possible values 1 or 2
     *
     * @return false if at least wrong parameter given or generator already in stop state
     */
    bool StopPWM(int num);

    /**
     * \brief Get PWM generator state if it is in a Start state
     * Method can be called in any time.
     *
     * @param num - output number - possible values 1 or 2
     * other parameters are output references to paramteres with same names in @ref StartPWM
     * they are valid only if true returned
     *
     * @return false if num parameter is wrong or generator is in stop state
     */

    bool GetPWM(int num, int& frequency, int& high, int& low, int& repeats, float& duty_cycle);

What max frequency value must be supported?

iluxa commented 4 years ago

according to tests sending AOUT4.raw< SPI command takes about 17ms, so the maximum frequency (1/T_total) is about 30Hz @mkh-welsch is that enough?

mkh-welsch commented 4 years ago

why is the maximum frequency dependent on the SPI transmission? the command is only transmitted once.

iluxa commented 4 years ago

To enable mode DACsw command need to be send Then for each switch between high/low level AOUT3.raw< or AOUT4.raw< command need to be send via SPI.

iluxa commented 4 years ago

output_pwm need to be implemented on firmware side to make pwm independent on the SPI transmission

IngoKaiser commented 4 years ago

@begodev please have a look on implementation of a output_pwm in firmware. any questions?

begodev commented 4 years ago

Yes, I think it is better to implement it in the firmware... For timing accuracy one of the hardware timers can be used for example (if its required). What does the output number parameter mean?

mkh-welsch commented 4 years ago

the channel number. only 2 of the 4 outputs can be set to manual mode.

begodev commented 4 years ago

According to TimeSwipe/docs/Communication/CommunicationProtocol.md conception I propose the following variables to control the PWM:

For the first chanel: domain PWM1, for the second - PWM2

the domain variable holds on/off state itself:

for example to start PWM1: PWM1<1, to stop: PWM1<0. get actuall state of PWM1: PWM1>

to control the waveform:

PWMx.frequency PWMx.high PWMx.repeats PWMx.duty_cycle

variables has the same meaning like was proposed above.

Is it suitable, what do you think?

IngoKaiser commented 4 years ago

That’s almost fine. Just what about start/stop? How does that relate to PWMx.repeats?

begodev commented 4 years ago

That’s almost fine. Just what about start/stop? How does that relate to PWMx.repeats?

I think it should give a burst of pulses=repeats, and then automatically switch to "stopped" state, or generate PWM continuously if repeats=0 for example.

Maybe it is better to rename "repeats" into "periods"

Before start all of the parameters should be set. Or previously set parameters will be used.

Then just send PWM1<1 - to start the generation. If PWMx.repeats=10 for example a burst of 10 pulses will be generated and then PWM will go to "stopped" state and PWM1> returns"0"

Also the function can be implemented as one command if it necessary (like function above) or js command can be used to set all parameters in one request.

iluxa commented 4 years ago

@begodev can you please explain what needs to be reviewed here

begodev commented 4 years ago

@iluxa the new firmware API can be tested according to CommunicationProtocol.md + generation (PWM waveform) in software polling mode like itis now

iluxa commented 4 years ago

pushed commit with PWM implementation in the driver and pwm example application: https://github.com/panda-official/TimeSwipe/commit/11acf5d6416d0c592e859072251f0b1846a981f5

@begodev Is anywhere firmware available with PWM features to test?

begodev commented 4 years ago

Is anywhere firmware available with PWM features to test?

PWM firmware is currently available @10.0.0.12

iluxa commented 4 years ago

Tested pwm API on 10.0.0.12

Everything looks ok, except: for some PWM commands if CBcmSPI.receive called right after CBcmSPI.send (without sleep or 3rd party io call like print to stdout between two calls) receive returns false. How to check: call pwm_static with get parameters and with get --trace-spi parameters actual pwm_static is at 10.0.0.12/root/ilya/issue_47

begodev commented 4 years ago

Actually CBcmSPI.receive just returns the bufer with responce filled by CBcmSPI.send, so shouldn't be any problem or influence of sleep or printf calls if send and receive are runing the same thread...

iluxa commented 4 years ago

Actually CBcmSPI.receive just returns the bufer with responce filled by CBcmSPI.send, so shouldn't be any problem or influence of sleep or printf calls if send and receive are runing the same thread...

Yes it shouldn't but it exists, it can be because of compiler optimizations, maybe some kind of memory barrier should be inserted at the end of send function.

begodev commented 4 years ago

I looked into board_iface.hpp: "true" and "false" are not allowed in simple commands. It is some mixup with JSON. "true" and "false" came from it. But initially they are processed by std::stoi, so they should be "1"=true (bool) and "0" =false (bool). Otherwise !stoi exception is returned.

begodev commented 4 years ago

So there are two independent parses exist...I can add "true" and "false" support along with "1" and "0" to the older one as a solution...

iluxa commented 4 years ago

I looked into board_iface.hpp: "true" and "false" are not allowed in simple commands.

true and false are not present in the last commit of the branch

test_spi_pwm.zip

Attached test_spi_pwm compiled with optimization (-O2) for aarch64 prints out error 8 times from 10 runs on 10.0.0.12. Binary path is /root/ilya/issue_47/test_spi_pwm

begodev commented 4 years ago

The problem can also be that CS (chip select) needs some time to rise and fall physically. It depends on circuit RC values. I put additional delay after CS rise at the end of the transfer which corresponds to one period of communication frequency (20 microSeconds)

iluxa commented 4 years ago

Tested it, pwm sample working without issues

begodev commented 4 years ago

think has to be tested also with a stepper motor driver. For low frequencies it should be ok, bu t for higher maybe hardware timer ticks is required...

IngoKaiser commented 4 years ago

@begodev can you specify expected frequency level?

begodev commented 4 years ago

PWM half-period switch time is about 14uS now. So when only both PWMs are running they can give up to 10kHz(superloop execution time of 30uS is included) if no other tasks are running. But now the frequency is limited by sys timer accuracy =1mS=1kHz. Other tasks running can decrease the maximum frequency. But I have to measure the time consumption in worst case for each task in the superloop. Tommorow I can look the oscilogramm of PWM...

begodev commented 4 years ago

Or otherwise I can just drive them from the hardware timer interrupt routine. What maximum frequency is required for the motor driver?

IngoKaiser commented 4 years ago

@begodev Should run with dma anyway to not have an impact on other features and backwards.

thomaseichhorn commented 4 years ago

The frequency we need depends a bit on the load which will be placed on the motor. As a ball-park figure, we will most certainly need at least 1kHz. The motor driver can handle up to ~20 kHz, so I don't think we will need anything beyond that.

begodev commented 4 years ago

Then I think Ingo is right, DMA support has to be added. Otherwise it seems the CPU load becomes quite high...

IngoKaiser commented 4 years ago

@begodev How long will you need - didn’t think one would do it differently 😜

begodev commented 4 years ago

I just choose the simplest and fastest solution, because initially it was supposed to generate PWM by external commands what cannot be fast and I went further along this.

begodev commented 4 years ago

@begodev How long will you need

I'll try to finish this week...hopefully

begodev commented 4 years ago

The generation of 20 kHZ was achieved with intermediate solution of using 1 hardware 32bit timer per PWM. Because the timer module is required to trigger DMA channels anyway. So it is one of the parts. In complete scheme I think 2 DMA channels, 1 triggering timer and 1 period(repetions) counter driving by the event system is required to run PWM without using CPU. At the moment at with maximum PWM load (both running at 20kHZ) the CPU usage will be ~25%. I think it is not bad. And those light-weight timer interrupt routines can be further optimized. If this satisfactory we can stop at this point or test PWM for a while. Otherwise we can continue with DMA schemes...

begodev commented 4 years ago

DMA generation mode is added. Now both generation schemes are available depending on CDacPWMht::mode parameter in CDacPWMht ctor

Limitations: 1) PWMx.repeats <= 65535 2) Changing of PWMx.repeats during generation running will not affect current generation but will be applied for the next one after restart.

begodev commented 4 years ago

What can be done else:

  1. full-weight Event System module (not implemented)
  2. some enhancement in CSamTC class (more interface methods encapsulating hardware details)