Chief Delphi - Delay code isn't delaying.

Chief Delphi (http://www.chiefdelphi.com/forums/index.php)

- Programming (http://www.chiefdelphi.com/forums/forumdisplay.php?f=51)

- - Delay code isn't delaying. (http://www.chiefdelphi.com/forums/showthread.php?t=66912)

Re: Delay code isn't delaying.

Quote:

Originally Posted by Alan Anderson (Post 736299)

I too find the highly compact style a hindrance to readability. I actually reformatted the relevant lines in order to make it easier for me to see what the code was trying to do.

What would you suggest I do to make it more readable?

Re: Delay code isn't delaying.

It looks like you might be being bitten by C18's habit of doing single byte arithmetic. In your if statement cast WAITING_TIME and LOOPS_IN_A_SECOND to int.

For example:
if(auton_counter < (int) WAITING_TIME * (int) LOOPS_IN_A_SECOND){ //wait

C18 is most likely currently treating them as single byte numbers - so 8 x 38 is 304, which overflows to become 48. (Which would only give you about a 1 second wait.)

Re: Delay code isn't delaying.

Quote:

Originally Posted by rdaugherty (Post 736310)

Thanks, I'll do that and give it a shot on Thursday.

Re: Delay code isn't delaying.

I know this sound trivial, and it "should" work the way you have it, but....

Try changing

Code:

if(auton_counter < WAITING_TIME * LOOPS_IN_A_SECOND)

Code:

if(auton_counter << (WAITING_TIME * LOOPS_IN_A_SECOND))

I've been bitten by this a couple times.

Re: Delay code isn't delaying.

Quote:

Originally Posted by billbo911 (Post 736363)

if(auton_counter << (WAITING_TIME * LOOPS_IN_A_SECOND))

The double less-then signs will not do what you intend (they will shift auton_counter left by WAITING_TIME * LOOPS_IN_A_SECOND, which will end up being 0).

Re: Delay code isn't delaying.

Quote:

Originally Posted by rdaugherty (Post 736310)

This is the correct solution.

Re: Delay code isn't delaying.

Quote:

Originally Posted by Guy Davidson (Post 736307)

What would you suggest I do to make it more readable?

Spread stuff out a bit. Add white space. Put matching braces in the same columns. That sort of thing. Where you have this:

Code:

        if(auton_counter < WAITING_TIME * LOOPS_IN_A_SECOND){ //wait

                auton_counter = auton_counter + 1;

                pwm13 = pwm14 = pwm15 = pwm16 = 127;

                DEBUG(("Counter :%d\r\n",(int)auton_counter));

        }else{ //go

                e1 = Get_Encoder_1_Count(); //from encoder.h, left side

...

I would change it to

Code:

        if (auton_counter < WAITING_TIME * LOOPS_IN_A_SECOND)

        { //wait

                auton_counter = auton_counter + 1;

                pwm13 = pwm14 = pwm15 = pwm16 = 127;

                DEBUG(("Counter :%d\r\n",(int)auton_counter));

        }

        else

        { //go

                e1 = Get_Encoder_1_Count(); //from encoder.h, left side

...

All I did was add spaces, tabs, and newlines.

If you prefer terseness, here is a common idiom for such counters:

Code:

        if (auton_counter++ < WAITING_TIME * LOOPS_IN_A_SECOND)

        { //wait

                pwm13 = pwm14 = pwm15 = pwm16 = 127;

                DEBUG(("Counter :%d\r\n",(int)auton_counter));

        }

        else

        { //go

                e1 = Get_Encoder_1_Count(); //from encoder.h, left side

...

I have learned not to rely on preprocessor arithmetic to get things right. It took us two regionals to figure that out this year. Instead of a nice, readable travel ( 150 * INCH ); (INCH is 25) we have to say travel ( 3750 ); in order for everything to work the way we expect.

Re: Delay code isn't delaying.

Quote:

Originally Posted by Alan Anderson (Post 736465)

Spread stuff out a bit. Add white space. Put matching braces in the same columns. That sort of thing. Where you have this:

All I did was add spaces, tabs, and newlines.

If you prefer terseness, here is a common idiom for such counters:

Thanks. I'll keep that in mind when I get around to writing more code.

Quote:

Originally Posted by Alan Anderson (Post 736465)

It could actually be the same problem. If, as rdaugherty suggests, the compiler tends to use byte arithmetic if both operands are bytes and it isn't explicitly told otherwise, then in your multiplication too, it would end up casted into a byte, I think. I'm planning to test this theory in Atlanta, once I have robot access again.

Re: Delay code isn't delaying.

Since integer promotion isn't enabled by default, and those preprocessor macros are effectively just search-and-replace, the calculations would not give the expected result. Casting them correctly and using parentheses generously should fix this, as previously stated.