Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update drv_pwm.c #153

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Update drv_pwm.c #153

wants to merge 4 commits into from

Conversation

Crashpilot1000
Copy link

Detection & fix for the "8 channels in 18ms Frsky problem" see http://diydrones.com/profiles/blogs/why-frsky-cppm-signal-is-so-disappointing . Note: failsafeCheck not altered!

Detection & fix for the "8 channels in 18ms Frsky problem" see http://diydrones.com/profiles/blogs/why-frsky-cppm-signal-is-so-disappointing . Note: failsafeCheck not altered!
bool sync = diff > 2700; // rcgroups.com/forums/showpost.php?p=21996147&postcount=3960 "So, if you use 2.5ms or higher as being the reset for the PPM stream start, you will be fine. I use 2.7ms just to be safe."
last = newval;

if(frsky_18patch) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spacing between if and (

@trollcop
Copy link
Member

If you can correct the above stuff, I'll merge this.
It just looks ugly if I merge this and then have to do another commit just to fix spacing/etc

@Crashpilot1000
Copy link
Author

I will do another pull request with a better formated version now.

Reformated the code, tested it again - works. The failsafeCheck is not changed and probably needs further attention from the BF folks.
@trollcop
Copy link
Member

trollcop commented Aug 1, 2014

oK!

On Fri, Aug 1, 2014 at 5:37 AM, Crashpilot1000 notifications@github.com
wrote:

I will do another pull request with a better formated version.


Reply to this email directly or view it on GitHub
#153 (comment).

@Crashpilot1000
Copy link
Author

Just doing a shorter version it is as good but cleaner. Have to do some testing. Will commit it when done.

@trollcop
Copy link
Member

trollcop commented Aug 1, 2014

OK

On Fri, Aug 1, 2014 at 11:27 AM, Crashpilot1000 notifications@github.com
wrote:

Just doing a shorter version it is as good but cleaner. Have to do some
testing. Will commit it when done.


Reply to this email directly or view it on GitHub
#153 (comment).

Shorter and working.
@Crashpilot1000
Copy link
Author

That is it from my side. Plz test it out as well :)

Besides the Frsky 18ms fix it also verifies the channelnumbers, that will most probably also fix some hiccups some ppl see. I can not verify this because I don't see those hiccups with my gear. BF failsafeCheck logic unchanged may need attention.
@Crashpilot1000
Copy link
Author

The last commit also does a better check on the channel order, there is a good chance it fixes those hiccups some ppl see (can not check it, because no hiccups with my gear).

@fiendie fiendie force-pushed the master branch 2 times, most recently from 0e8347c to fed0125 Compare October 24, 2014 11:10
@ghost
Copy link

ghost commented Mar 13, 2015

Can one of the admins verify this patch?

@fiendie
Copy link
Member

fiendie commented Mar 13, 2015

Just a whitespace issue. We could merge this.

@kh4
Copy link
Contributor

kh4 commented Mar 13, 2015

I do not like this patch.

It adds a workaround to a specific problem that has a proper fix (27ms firmware).

Also as it generates 'sync' from thin air so it is possible to drop out of sync when the input is without sync for a longer period of time due to glitch or what ever.

As per the channels order, a more simple way is to just set chan=255 when an invalid pulse is detected causing the rest of the frame being ignored.

@kh4
Copy link
Contributor

kh4 commented Mar 13, 2015

To be even more safe against glitches the whole frame should be ignored if any of the channels fall out of the 750-2250 range, setting chan to 255 would just ignore input after the glitch.

For better resiliency the captures should be saved to 'temporary' table and only copied to captures when next sync is detected with proper number of channels sampled. This approach will also ignore the 'corrupt frames' from a 18ms frsky receiver.

@fiendie fiendie added the bug label Mar 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants