Writing output values to pins back to back seems buggy, digitalwritefast() too

jraskell:
You should do some more research. I don't believe there is a mechanical relay in existence that operates fast enough to make digitalWrite()'s speed a problem. Most take on the order of 10-20ms to switch. There may be some fast ones out there that could take as little as 1-2ms, but that's probably about as fast as you're going to get without switching to solid state relays, and that's still around several hundred to a thousand times slower than digitalWrite().

I'm not sure how you got there, so let me try to explain better than I previously did how/why I started looking around and calling digitalwrite() slow.

I have found that if I use code similar to this:

digitalwrite(2, high)
digitalwrite(3, low)

The second line does not execute/activate/what-have-you. The end result, ie the symptom, is that the relay and LED tied to that pin to not close (it is active low) and turn on, respectively.

I don't know why that is, it seems kid of weird/silly to me, as I've seen code where people do something like that and it seems to work, so I don't know what is going on, but I tried digitalwritefast() and that fixed it in most cases. However, in the case of:

    if (millis() > (whatTimeIsIt + idleMotorKill) ){
      //digitalWriteFast(3,HIGH);
      //digitalWriteFast(1,LOW);      
      engineKill(); // digitalWrite(Relay_1, RELAY_ON);
      brakesEngage();
    } // test millis()

I see a similar error condition to using digitalwrite(): the second digitalwritefast() command in the engineKill() function does not appear to execute. However, the next digitalwritefast() in the brakesEngage() function called does activate as I expect. I suspect this might have to do with the "slow-down" or compatiblity, or inherent safeness of the digitalwritefast() library, and for whatever reason digitalwritefast() is going back to digitalwrite() "speeds", assuming it's even a speed issue. If digitalwrite() is fast enough for most use, then there is something else going on here that I don't understand.

I would like to know why/how this is the case so I can work around it, or better understand how to work with Arduino (this is still my first arduino project by the way) so I don't run into this sort of thing.

So what do you mean when you say that those lines should not go together?

Which pins does the HardwareSerial class, of which Serial is an instance, use? When you determine that, you'll see why the two lines do not belong in the same sketch.

allanonmage:
I have found that if I use code similar to this:

digitalwrite(2, high)

digitalwrite(3, low)

Can you post a simple sketch that illustrates this behaviour?

PaulS:

So what do you mean when you say that those lines should not go together?

Which pins does the HardwareSerial class, of which Serial is an instance, use? When you determine that, you'll see why the two lines do not belong in the same sketch.

Look, I appreciate your help and don't want to piss you or anyone else off, but trying to use the Socratic method to force me to address a potential issue with a piece of commented out code.... that's a little off topic. I commented that piece out early in the process because I realized it was on the same pin as a serial port. In fact, that is what led me to spell out the

// turn relay 1 (on pin 2) off, part 1 of disengaging the engine

comments around my functions so I could keep them straight.

If you or someone else would be so kind as to confirm if activating digitalwrite()'s on sequential pins is supposed to work, I would much appreciate it. At this point, I'm really not sure if it is or is not supposed to work.

@dxw00d your comment asking for a sketch came in as I was writing this one. I'll post one here in a few minutes, maybe even a video so you can see how it's wired up too.

If you or someone else would be so kind as to confirm if activating digitalwrite()'s on sequential pins is supposed to work, I would much appreciate it.

Of course it is supposed to work. And, it does. You are having other issues.

Commenting out code that doesn't work is silly. There is a reason you have a delete key.

In fact, that is what led me to spell out the
Code:

// turn relay 1 (on pin 2) off, part 1 of disengaging the engine

comments around my functions so I could keep them straight.

Creating variables to contain pin numbers would let you avoid that.

const int relay1Pin = 2;

Then, you just

digitalWrite(relay1Pin, HIGH);

to deal with that relay.

Self-documenting code is a good thing.

allanonmage:
I noticed that the 2nd change, when it was supposed to occur, instead produced a visible dimming of the LED associated to that output.

You're more than likely watching the led turn on and off faster than you can see.
It only takes 24 FPS to fool human eyes.

Video of what I am doing here:

I go through various different and repetitive tasks in the video so you can get a feel for what's actually happening. Not much on the sound front, mostly just background noise, but you should be able to hear me engage or disengage the switch and the relays clicking.

To reiterate, the problem is that after the timeout period there wind up being 3 LEDs on. The one on the end should turn off when the timeout occurs and the middle two turn on. We see that a manual turn off of the transmitter allows for this, yet when the timeout value is hit, something different and unexpected occurs.

I need help understanding that different/unexpected part.

I made a public facebook photo album for some pics I snapped of what I've got.

https://www.facebook.com/media/set/?set=a.10151115394025498.485312.720010497&type=1&l=83f051ba3d

If you want real help then you have to be very clear about what your problem is. I looked at the video and there is no sound. No explanation of what you are doing or what you are expecting to see. All it looked like to me was that you clicked a switch and LEDs went on and off, with no clue as to if this is what you wanted or what you wanted.

To be clear you need to post the code you are using with all those commented out instructions removed they are a distraction. Then you need to say what you see happening and what you expect to happen.
Better still write a smaller sketch that illustrates your problem.

I can tell you now that writing data to pins in not buggy and that if you are using relays the time difference between a digitalwritefast() and a normal digitalWrite() is totally insignificant.

allanonmage:
I don't understand what you are talking about, so I googled "digitalwritefast serial.begin" and found no mention of digitalwritefast and serial.begin having any problems together.

PaulS was just saying, in a rather humorous way IMHO, that wanting to do things "fast" and combining that with serial comms, is a contradiction in requirements.

It's like saying you want to organize a fast turtle race.

And you can Google "fast turtle race" and not necessarily find any problems per se. Except people laughing at the very idea.


allanonmage:
I have found that if I use code similar to this:

digitalwrite(2, high)

digitalwrite(3, low)




The second line does not execute/activate/what-have-you.

Stop right there. First, your code is wrong in about 5 points. If you are going to say code X doesn't work, post code X, and prove it. Missing semicolons, "high" instead of "HIGH", incorrect capitalization of digitalWrite and so on.

So you are saying this code:

digitalWrite(2, HIGH);
digitalWrite(3, LOW);

Does not turn pin 2 high and pin 3 low?

Well that's just not correct. So stop trying to use digitalWriteFast and wandering off into discussions about the Socratic method, and concentrate on demonstrating this particular claim.

Have you set the pins to outputs? First important point. If not, we can skip the rest.

In fact, post your whole code, not just snippets, because that hides whatever-it-is you are doing that causes the problem that you perceive.

If you or someone else would be so kind as to confirm if activating digitalwrite()'s on sequential pins is supposed to work, I would much appreciate it.

Do you seriously think Atmel would sell any chips if you could not turn on or off two things in a row reliably? Of course it is supposed to work.

PaulS was just saying, in a rather humorous way IMHO, that wanting to do things "fast" and combining that with serial comms, is a contradiction in requirements.

I was more that the code showed pin 1 being set by digitalwritefast() while the Serial.begin() implies you are using serial communications and that uses pin 0 and 1.
So you would be using the same pin for two different incompatible reasons.

Hmm, I see you posted code further up. Is that current? Try using your constants consistently, not:

  pinMode(Relay_1, OUTPUT);   
...
  digitalWriteFast(2,HIGH); // turn relay 1 (on pin 2) off, part 1 of disengaging the engine

That's confusing.

Since you have serial comms turned on, how about some debugging prints? That will confirm your code is doing what you think, which I strongly doubt.

Long story short you guys got me riled up (aggravation is a wondrous motivator, eh?) and in getting the code ready to send in here and making a video explaining things step by step, I saw the flaw in my logic. It was very similar to the first problem I had with the time out with millis() (this was before I wrote in for help), and I should have seen it sooner.

This is where I am at now, with much comments removed and digitalWriteFast() not in use.

#include <ServoDecode.h>
#include <digitalWriteFast.h>

char * stateStrings[] = {"NOT_SYNCHED", "ACQUIRING", "READY", "in Failsafe"};
  
// Define readable non-backwards states for relays.
// This is because relays are active LOW.  That means you can ground the pin and activate the relay.
const byte RELAY_ON = 0;
const byte RELAY_OFF = 1;

// Give each relay friendly name and map to Arduino Digital I/O pin number
const byte Relay_1 = 2;  // wire 1 of the engine kill actuator.  Pulse to allow engine to run.
const byte Relay_2 = 3;  // wire 2 of the engine kill actuator.  Pulse to kill engine.
const byte Relay_3 = 4;  // brake controller
const byte Relay_4 = 5;  // currently unused

const unsigned long idleMotorKill = 5000; // delay to kill the motor
long whatTimeIsIt = 0;
boolean timeOut = false;

void setup(){
  
  //ServoDecode setup
  ServoDecode.begin();
  ServoDecode.setFailsafe(3,1234); // set channel 3 failsafe pulse  width
  
  // Relay Setup Initialize Pins so relays are inactive at reset
  digitalWrite(Relay_1, RELAY_OFF);
  digitalWrite(Relay_2, RELAY_OFF);
  digitalWrite(Relay_3, RELAY_OFF);
  digitalWrite(Relay_4, RELAY_OFF); 
  
  // THEN set pins as outputs
  pinMode(Relay_1, OUTPUT);   
  pinMode(Relay_2, OUTPUT);  
  pinMode(Relay_3, OUTPUT);  
  pinMode(Relay_4, OUTPUT);    
  delay(1000); // Check that all relays are inactive at Reset
  
  // Prevent motor from starting and engage breaks
  engineKill();
  brakesEngage();

} // end setup()


void loop(){

  if(ServoDecode.getState()!= READY_state) { // transmitter is off or out of range
    engineKill(); // kill motor
    brakesEngage(); // engage brakes
    whatTimeIsIt = 0; // used as a semaphore
    timeOut = false; // resets this semaphore
  } // end if() transmitter not ready
  
  if(ServoDecode.getState()== READY_state) {
    if(whatTimeIsIt==0){ // tests for if it's the first loop through of the ready state
      whatTimeIsIt = millis(); // if it's the first loop through, make a note of the time
    } // end if() testing for first loop through
    
    if(!timeOut){
      engineAllow(); // allow motor to be cranked
      brakesDisengage(); //disengage brakes
    } // if the transmitter has not timed out

    if (millis() > (whatTimeIsIt + idleMotorKill) ){ // if 5 seconds has passed since entering the ready state, kill the motor and engage the brakes
      engineKill();
      brakesEngage();
      timeOut = true;
    } // end test for timeOut
  } // end if() transmitter ready
  
} // end main loop()

void engineKill(){
  digitalWrite(Relay_1, RELAY_OFF); // just in case it was on for some random reason; prevents frying relays and/or actuators
  digitalWrite(Relay_2, RELAY_ON); // eventually pulse it, but for now just latch it
} // end engineKill

void engineAllow(){
  digitalWrite(Relay_2, RELAY_OFF); // just in case it was on for some random reason; prevents frying relays and/or actuators
  digitalWrite(Relay_1, RELAY_ON); // eventually pulse it, but for now just latch it
} // end instantEngineKill

void brakesEngage(){
  digitalWrite(Relay_3, RELAY_ON); // engages the brakes by closing the circuit to power them
} // end brakesEngage

void brakesDisengage(){
  digitalWrite(Relay_3, RELAY_OFF); // disengages the brakes by opening the circuit
} // end brakesDisengage

Notice the new boolean timeOut semaphore and additional conditional logic [ if(!timeOut){} ]within the loop; that's what I was missing. As for everyone who was asking about code, the whole thing was in the OP this whole time. Yes I was running the same code as in the OP when I shot the first video. Sorry about the lack of audio, I was at work trying to be discreet about it and it seemed self evident of what I was doing based on the code.

Video is uploading now from my phone, it'll be in my channel. You can get an idea of what I am trying to do as I do explain the process of what I want to happen. It's taking it's sweet time uploading, so I'll edit this post after the fact with a direct link to it. I tried to talk slow, but my phone has audio/video sync issues of about a fraction of a second; enough to be annoying.

Direct link to the video:

Ah that makes sense now. I'm not sure how clear I was in that other thread and this one, but I'm not familiar with microcontrollers; I typically work with/on PCs, or in the case of my job, a completely proprietary system with proprietary software. What I'm getting at is my implicit communication skills in this world are going to be a while in developing. Kind of like how sarcasm is lost on young children. I do try to do some research and/or trial & error before I ask questions though.

No, my point was, and up till I just figured out my logic error, always was, that the second line, ie "digitalWrite(3, LOW);" was not executing. In this case it just appeared to not be executing when it actuality it was; without proper conditional logic another function was just writing all over it and it was looping fast enough that it appeared to not work. The dim LED was somewhat of a clue, albeit it too faint for me to pick up on.

Is it not a good idea to use this library? Or does this fall under the fact that if it doesn't work normally it won't work optimized?

I'm not trying to be rude, but I'm going to be blunt here: it's in the OP. In a code block labeled "Here's the test code I'm working on:"

When I'm new to a piece of tech and it's not working the way I want, I try removing unsubstantiated assumptions, guesses, or things I "think" I know. As not having a clue, I could only assume that was the case, as I had little to no proof it was one way or another. In fact, I even went so far as to postulate something halfway plausible as to why it might not work the way I originally thought it would.

Someone once said that you can't actually debug your own code because if you're not capable enough to see the problem, you can't see it. Or something like that. It was smart & deep. Anyways, I thought I had double and triple checked my code at that point, which is why I started to wonder about other things.

I know microcontrollers are more limited than PCs in some aspects, but I don't quite understand the hows and whys and limits of that concept, so I looked around, came up with a guess, and asked if I was close or way off. Seems that this time I was way off. That's what I meant by:

I'm not sure if I did something stupid, or just didn't take into account the nature of the architecture.

In this case it was the former. For reference, my first foray into my company's language landed me in the latter.

Someone once said that you can't actually debug your own code because if you're not capable enough to see the problem, you can't see it.

Not always true in more than one way.

You may be tired and need some rest.

or

You may be fixated and need to take a clean view, which is very common, very left-brain.

OTOH your original idea may be flawed and then it's up to you to find that out.

allanonmage:
In this case it just appeared to not be executing when it actuality it was; without proper conditional logic another function was just writing all over it and it was looping fast enough that it appeared to not work. The dim LED was somewhat of a clue, albeit it too faint for me to pick up on.

Yes, that can happen. If possible, use a logic analyzer or oscilloscope to check whether an output is on all the time or switching on and off rapidly.

allanonmage:
Is it not a good idea to use this library? Or does this fall under the fact that if it doesn't work normally it won't work optimized?

I'm suggesting that you were flying off on a tangent here. The problem was not with digitalWrite.

allanonmage:
I'm not trying to be rude, but I'm going to be blunt here: it's in the OP. In a code block labeled "Here's the test code I'm working on:"

Sorry. I acknowledged that in my next post.

allanonmage:
Someone once said that you can't actually debug your own code because if you're not capable enough to see the problem, you can't see it. Or something like that. It was smart & deep. Anyways, I thought I had double and triple checked my code at that point, which is why I started to wonder about other things.

I understand, but it would have been good to make a test case with a half-dozen lines of code that simply turned pins on and off in quick succession. That would prove/disprove your theory about timing. Once you establish that the processor can, in fact, turn pins on and off quickly you might have looked more closely at the surrounding logic.

Hi allanonmage

I don't need a video - just explane in simple words what you want to achive with your program.

I have looked at your program and this is how I interpretate it:

  1. Waite for ServoDecode to give READY_state.
  2. If READY_state: Set relays to running state.
  3. If !READY_state: Set realys to not running state, and prepare for a new run.
  4. If running state has been activitated for too long: Set relays to not running state and do nothing forever.

Is this correct?

-Fletcher

I think you missed a few posts. He found the source of his problem himself.

Don't forget when you're debugging you can insert a sizeable delay after each output to see if anything is happening or use LED 13 as an indicator that you got to "here" in the code.
It is one of the benefits of delay(), it stops everything (unless you are using a time slicing OS).

// ie
digitalWrite(2, HIGH);
delay(1000);
digitalWrite(3, LOW);
delay(1000);
// etc

k

I wouldn't worry about OS on an Arduino.