Delay code isn't delaying.

[Guy Davidson]

I'm working with Kevin Watson's 3.0 compatible FRC framework. I have code in the begining of my Autonomous() function that should cause the robot to wait, based on a few constants defined in Autonomous.h. It doesn't seem to work, and just takes off immediately. Could it have something to do with the fact that the field control system spends a moment in teleop before jumping to autonomous in the begining?

I attached a .zip archive with my entire workspace.

Thank you for helping.

-Guy Davidson

[Alan Anderson]

Might it have something to do with this line in autonomous.h?
#define WAITING_TIME 0

[Guy Davidson]

That line is commented out as part of this block:

/*
#define LOOPS_IN_A_SECOND 38
#define WAITING_TIME 0
#define TICKS_IN_A_FOOT 156 //163 * 23 / 24
#define DRIVE_FORWARD_1 10 //45 //in ft
#define DRIVE_FORWARD_2 10 //in ft
#define DRIVE_FORWARD_3 40 //in ft
#define DRIVE_FORWARD_4 10 //in ft
#define TURN_LEFT_1 65 // in diffs
#define TURN_LEFT_2 57 // in diffs
#define ANGULAR_INCREMENT 5 //in diffs
#define LINEAR_INCREMENT 156 //163 * 23 / 24
#define POSITION_INPUT_1 rc_dig_in17 //0 && 0 = LEFT, 1 && 0 or 0 && 1 = CENTER, 1 && 1 = RIGHT
#define POSITION_INPUT_2 rc_dig_in18 //0 && 0 = LEFT, 1 && 0 or 0 && 1 = CENTER, 1 && 1 = RIGHT
*/

The copy I hope it's using is defined a few lines later:

#define LOOPS_IN_A_SECOND 38
#define WAITING_TIME 8

Is there any way that's causing issues?

[Alan Anderson]

I didn't think the preprocessor cared about comment lines. Are you getting any compiler errors or warnings about trying to redefine a symbol?

[Guy Davidson]

Nope. No errors or warnings what-so-ever, as far as I can see.

[iTHOS=awesome]

The best thing to do for delays is make a counter that does everything. Here is a few lines that I used for our robot in the Tacoma Regional:

int timer = 0;

if(timer < 72)
/* 72, if I remember right, is about 2 seconds, this number can be adjusted however you need it to be */
{
timer ++;
}
else
{
// This is where the remainder of autonomous functions occured
}

[Ctrl Alt Delete]

It cycles 40 times a second right? So you can just make a function that returned i / 40 so you can say if(timer < 2) which would be the actual amount of seconds. I know it's basic math to just multiply by 40, but it makes the code a little more readable for someone that doesn't know that.

[Guy Davidson]

That's pretty much what I have there.

[iTHOS=awesome]

That's pretty much what I have there.

if that is what you are doing, I don't know what the issue would be. Can you copy and paste your delay code here so I (or anyone else) can help you?

[Guy Davidson]

It's in autonomous.c in the attached archive. Since it's also based on stuff in autonomous.h, I left it in an archive. Would it help if I copy both .c and .h here?

[Ctrl Alt Delete]

The only thing I can think of now is changing this:

if(auton_counter < WAITING_TIME * LOOPS_IN_A_SECOND)

to this:

if(auton_counter < (WAITING_TIME * LOOPS_IN_A_SECOND))

Maybe try printing out WAITING_TIME and LOOPS_IN_A_SECOND and see what it's outputting.

[Joe Ross]

What does your debug statement say for the value of counter?

[iTHOS=awesome]

The code is fairly confusing for me as to what you are trying to do. The way I learned programming makes this very hard to read, no offense. I just can't figure out what is wrong with it.

[Alan Anderson]

The way I learned programming makes this very hard to read, no offense.

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.

I don't remember who said it, but I agree with the philosophy that says a programmer's job is to write documentation that compiles into a working program.

[Guy Davidson]

What does your debug statement say for the value of counter?

That's the next thing I was going to check. I put it in there during our last competition in hopes of getting around to test it there, and I haven't been able to yet. I'm hoping to do that early Thursday morning in Atlanta.

[Guy Davidson]

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?

[rdaugherty]

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.)

[Guy Davidson]

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.)

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

[billbo911]

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

Try changing
if(auton_counter < WAITING_TIME * LOOPS_IN_A_SECOND)

to

if(auton_counter << (WAITING_TIME * LOOPS_IN_A_SECOND))

I've been bitten by this a couple times.

[Joe Ross]

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).

[Jared Russell]

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.)

This is the correct solution.

[Alan Anderson]

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:

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

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:


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.

[Guy Davidson]

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.

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.
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.

[tdlrali]

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.