Problem with timing using while and micros()

This code is supposed to send information by sending pulses on railPin with lengths depending on the values in toSend. sendBit() is supposed to wait about 55 or 95 μs for the high part of the pulse, and then sendBits() is supposed to wati for the remaining part of the 106 or 200 μs after sendBit has run. I uploaded this to an arduino nano, and every pulse seems to come out as about 12μs long. My guess is that it doesn't stop at the whiles that are supposed to stop execution until the time is up("probably problematic while nr.1 and nr.2"). Am I correct or is there some other problem? How can my code be fixed?

#define buttonPin1 6
#define railPin 3
#define button1IsPullup true
void setup() {
  if(button1IsPullup){
    pinMode(buttonPin1,INPUT_PULLUP);
  }
  else{
    pinMode(buttonPin1,INPUT);
  }
  if(button2IsPullup){
    pinMode(buttonPin2,INPUT_PULLUP);
  }
  else{
    pinMode(buttonPin2,INPUT);
  }
  pinMode(railPin,OUTPUT);
  pinMode(LEDPin, OUTPUT);
}
static int sendLength = 57;
static bool ToSend[] = {1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1, 0, 0,0,0,0,0,0,1,1, 0, 0,0,1,1,1,1,1,1, 0, 1,1,1,1,1,1,1,1 ,0 ,1,1,0,0,0,0,1,1 ,1 };
static byte OnMask = B00000001 << railPin;
static byte OffMask= (B11111110 << railPin) | (B11111111 >> (8-railPin));
void loop() 
{
  SendBits();
  while(digitalRead(buttonPin1) == button1IsPullup);
}
void SendBits()
{
  unsigned long snp;
  unsigned long bitLength;
  for(int i = 0; i < sendLength;i++){
    snp = micros();
    bitLength = ToSend[i] ? 106:200;
    SendBit(ToSend[i]);
    while(snp < micros()-bitLength);//probably problematic while nr.1
  }
}
void SendBit(bool value){
  unsigned long highLenth = (value) ? 55 : 95;
  unsigned long snp1 = micros();
  PORTD = PORTD | OnMask;
  while(snp1 < micros() - highLenth); //probably problematic while nr.2
  PORTD = PORTD & OffMask;
}

There are no useful comments.

Does this compile?

Yes, it does, except i made a mistake with the code above and didn't delete the 2nd button setup but did delete the #defines.
I will post the fixed code with comments.

#define buttonPin1 6
#define railPin 3
#define button1IsPullup true
void setup() {
//Sets up button1 as pullup if it is defined as such.
  if(button1IsPullup){
    pinMode(buttonPin1,INPUT_PULLUP);
  }
  else{
    pinMode(buttonPin1,INPUT);
  }
  pinMode(railPin,OUTPUT);
  pinMode(LEDPin, OUTPUT);
}
static int sendLength = 57;//number of bits to be sent.
static bool ToSend[] = {1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1, 0, 0,0,0,0,0,0,1,1, 0, 0,0,1,1,1,1,1,1, 0, 1,1,1,1,1,1,1,1 ,0 ,1,1,0,0,0,0,1,1 ,1 };//the bits to be sent
static byte OnMask = B00000001 << railPin;//mask that turns of railPin
static byte OffMask= (B11111110 << railPin) | (B11111111 >> (8-railPin));//mask that turn off railPin
void loop()
{
  SendBits();
  while(digitalRead(buttonPin1) == button1IsPullup);//wait until the button is pressed
}
void SendBits()
{
  unsigned long snp;
  unsigned long bitLength;
  for(int i = 0; i < sendLength;i++){
    snp = micros();//take a snapshot of micros at the beginnig of the bit
    bitLength = ToSend[i] ? 106:200;//set the length of the bit
    SendBit(ToSend[i]);
    while(snp < micros()-bitLength);//probably problematic while nr.1
  }
}
void SendBit(bool value){
  unsigned long highLenth = (value) ? 55 : 95;//set the length of the high part of the pulse
  unsigned long snp1 = micros();//take a snapshot of micros()
  PORTD = PORTD | OnMask;//turn on railPin
  while(snp1 < micros() - highLenth); //probably problematic while nr.2
  PORTD = PORTD & OffMask;//turn off railPin
}

One suggestion -

static byte OffMask = ~OnMask;

instead of

static byte OffMask= (B11111110 << railPin) | (B11111111 >> (8-railPin));

which was not commented, leaving it very cryptic.

Also this will break if you choose no pullup and don't change the polarity. You've used the variable in a completely different context:

  while(digitalRead(buttonPin1) == button1IsPullup);//wait until the button is pressed

What you mean is polarity, you are adding the pullup configuration as a convenience. But the result is terribly confusing. You should rename the variable.

Am I correct or is there some other problem? How can my code be fixed?

One way to test is to use delayMicroseconds() instead of your while statements.

You might, as a consideration, keep track of how long the code is taking to run and use the code length of time to adjust the uSecDelayTime.

code starts at 2000uS DelayTime

code time of execution 10uS (total time for a loop = 2010uS)

Adjust delay time DelayTime = 2000uS - 10uS to obtain a bit more precise execution + loop time.

while(snp1 < micros() - highLenth);

Let's assume snp1 = 1000, and highLenth = 100. The first time the while statement is tested, it becomes:

while (1000 < 1000 - 100);  // while (1000  < 900);

This is obviously false, since 1000 is NOT less than 900. The while statement will NEVER loop.

Simply printing the arguments to the while statement would have caught this in about 2 seconds...

Regards,
Ray L.

RayLivingston:

while(snp1 < micros() - highLenth);

Let's assume snp1 = 1000, and highLenth = 100. The first time the while statement is tested, it becomes:

while (1000 < 1000 - 100);  // while (1000  < 900);

This is obviously false, since 1000 is NOT less than 900. The while statement will NEVER loop.

Simply printing the arguments to the while statement would have caught this in about 2 seconds...

Regards,
Ray L.

Thanks for the answer, I don't know why I couldn't think it through :confused:

aarg:
One suggestion -

static byte OffMask = ~OnMask;

instead of

static byte OffMask= (B11111110 << railPin) | (B11111111 >> (8-railPin));

which was not commented, leaving it very cryptic.

Thanks for the idea, I will use that instead!

aarg:
Also this will break if you choose no pullup and don't change the polarity. You've used the variable in a completely different context:

  while(digitalRead(buttonPin1) == button1IsPullup);//wait until the button is pressed

What you mean is polarity, you are adding the pullup configuration as a convenience. But the result is terribly confusing. You should rename the variable.

I will consider adding a button polarity definition.