Code review request

[GoogleCodeExporter]

Hi Dmitry,

nice rewrite, it works very well!

I tried to add some PPM signal checks, but failed with adding a power off on a 
signal loss (see filter_ppm_data() and rx_ppm_callback()).

What should be added:
- Detect signal loss (not working with my patch)
- Require some valid ppm frames to arm the ESC
- Require some valid ppm frames to start the motor

Cheers,
Heiko

Original issue reported on code.google.com by h3ifri on 19 Aug 2012 at 11:18

Attachments:

• SignalCheck.patch

[GoogleCodeExporter]

Hi Heiko,

Thank you for your feedback! Currently I'm trying to implement safety features, 
requested by you in branch. And your feedback would be appreciated.
-PPM stream timeout - implemented, require some testing. 
-During arm we need at least one valid PPM frame. It is the same as in V 1 and 
RapidESC. Do you think is it sufficient? I do not mind adding some waits here, 
just not sure is it really needed.
-Wait_for_power_on() did not changed. But due difference between arm value and 
start value at least one valid PPM frame is required to pass this check. I 
would prefer not to add additional waits here, as current start-up procedure 
has ability to sync with rotating motor really quick. And it should be possible 
to re-start the motor really quick.
-Pull-up on rcp_in.  Initially it was set in AttachPPM(), but then was lost in 
free_spin(). I have decided to change free_spin() to turn off FET's one-by-one. 
This should preserve pull-up.

Regards,
Ziss_dm

Original comment by d.kayu...@gmail.com on 20 Aug 2012 at 3:53

[GoogleCodeExporter]

Original comment by d.kayu...@gmail.com on 20 Aug 2012 at 11:25

• Changed state: Fixed

[GoogleCodeExporter]

Hi Dmitry,

I will test your changes tomorrow.

Arming: I think one valid ppm frame can accidently happen. I tested r270 with a 
servo tester powerd by the ESC. The servo tester was set to 1400us before 
powering it up. The ESC was always aremd and the motor started spinning.

The reason for the waits in wait_for_power_on(): With V1, when the ESCs are 
already aremd and the FC is reseted with a reset button or by software (without 
powering down the ESCs), the motors somtimes start to spin for a very short 
time.

Cheers,
Heiko

Original comment by h3ifri on 20 Aug 2012 at 7:45

• Added labels: Priority-Low
• Removed labels: Priority-Medium

[GoogleCodeExporter]

Hi,

I have added wait for 50 frames and removed __delay_ms(250), so it feels the 
same as before. ;)

I remember that, MWC doing the same on reboot. But I thought there something 
wrong during initialization. As a workaround I have added wait for 15 frames, 
and looks like this fixed problem with MWC.

regards,
ziss_dm

Original comment by d.kayu...@gmail.com on 21 Aug 2012 at 12:19

[GoogleCodeExporter]

Hi!

Tested your latest code on the bench and everything worked very well. I will do 
some more tests on the weekend.

Is it possible with SDM to implement something like complementary PWM?

Cheers,
Heiko

Original comment by h3ifri on 21 Aug 2012 at 7:42

[GoogleCodeExporter]

Hi,

It should be possible to implement COMP-PWM. But I never heard, that somebody 
actually using it. ;)

regards,
ziss_dm

Original comment by d.kayu...@gmail.com on 22 Aug 2012 at 1:43

[GoogleCodeExporter]

Hi,

I use the V1 version with COMP_PWM enabled for month! Maybe you don't hear 
anything because it works perfect ;)

Cheers,
Heiko 

Original comment by h3ifri on 22 Aug 2012 at 4:20

[GoogleCodeExporter]

Original comment by d.kayu...@gmail.com on 28 Aug 2012 at 5:02

• Changed state: Verified