Can't toggle switch in this sketch

Projects Programming
1

Hello all,

I am trying to make the code below work, but i'm at a loss trying to toggle pin 4.
Pin 2 is connected to an rc receiver channel, when the rc signal goes from approx 988 to approximately 1014 pin 4 should toggle.
the stanby value for the rc signal is approx 988.
when i get this working i will add 17 more switch functions and use the arduino as a 18 channel multiswitch

if more info is needed please let me know.

thanks in advance.

#define MSpin 2
#define SW1 4

volatile long StartTimeMS = 0;
volatile long CurrentTimeMS = 0;
volatile long PulsesMS = 0;
int PulseWidthMS = 0;

unsigned long previousMillis = 0;  // delay variable

const long interval = 50;  // delay time in ms

int PSTBY = 1000;
int P1L = 1009;
int P1H = 1019;

int SW1State = 0;  // current state of the button

void setup() {

  Serial.begin(9600);
  pinMode(MSpin, INPUT_PULLUP);
  pinMode(SW1, OUTPUT);
  digitalWrite(SW1, LOW);
  delay(500);
  digitalWrite(SW1, HIGH);

  attachInterrupt(digitalPinToInterrupt(MSpin), PulseTimerMS, CHANGE);
}

void PulseTimerMS() {
  CurrentTimeMS = micros();
  if (CurrentTimeMS > StartTimeMS) {
    PulsesMS = CurrentTimeMS - StartTimeMS;
    StartTimeMS = CurrentTimeMS;
  }
}

void loop() {
  //only save pulse lengths that are less than 2000 microseconds
  if (PulsesMS < 2000) {
    PulseWidthMS = PulsesMS;

    Serial.println(PulseWidthMS);
  }
  noInterrupts();
  unsigned long currentMillis = millis();  // delay variable

  if (currentMillis - previousMillis >= interval) {  // check if the current time minus the previous time is less or equal to the value on line 12
    previousMillis = currentMillis;                  // if previous line is true, wait for amount of ms set on line 12

    if (PulsesMS < PSTBY)
      if ((PulsesMS > P1L) && (PulsesMS < P1H)) {
        if (SW1State == LOW)
          ;
        SW1State = HIGH;
      } else if (SW1State == HIGH)
        ;
    {
      SW1State = HIGH;
    }
  }
  interrupts();
}
2

First, bump up your baud rate to 115200. That will reduce any effect of the time it takes to print. Maybe not a problem yet, but just for the future.

Next, the ISR goes on CHANGE, so how do you know you are measuring the time the pulse HIGH, not LOW?

It's early and I can't run your code, just reading through the tiny window while in transit. So maybe you are getting reliable pulse widths. How do the printed values look?

This is odd code oddly formatted

    if (PulsesMS < PSTBY)
      if ((PulsesMS > P1L) && (PulsesMS < P1H)) {
        if (SW1State == LOW)
          ;
        SW1State = HIGH;
      } else if (SW1State == HIGH)
        ;
    {
      SW1State = HIGH;
    }

and is equivalent to this

    if (PulsesMS < PSTBY)
      if ((PulsesMS > P1L) && (PulsesMS < P1H)) {
   
        SW1State = HIGH;
      }

    SW1State = HIGH;

I think, again the tiny window but it looks like you have some stray semicolons in there and maybe are missing a pair of { braces } or two, review the syntax of the if/else statement and be certain the code is doing what you want.

I look better when i am in the lab, I am sure those lines are going for something but falling short.

HTH for now.

a7

3

This makes some sense

    if (PulsesMS < PSTBY) {
      if ((PulsesMS > P1L) && (PulsesMS < P1H)) {
        if (SW1State == LOW) 
          SW1State = HIGH;
        else SW1State = LOW;
      }
    }

If the pulses are short enough and also between P1L and P1H, toggle SW1State.

But if PulsesMS is less than PSTBY

int PSTBY = 1000;
int P1L = 1009;
int P1H = 1019;

it's never going to be between P1H and P1L. So that seems like it also somethig to look at.

a7

4

i tried some things with no luck based on your reply, i get good values in the serial monitor.
when i change my code to the below, it works but the but the connected led only is on when when i hold the button.

#define MSpin 2
#define SW1 4

volatile long StartTimeMS = 0;
volatile long CurrentTimeMS = 0;
volatile long PulsesMS = 0;
int PulseWidthMS = 0;

unsigned long previousMillis = 0;  // delay variable

const long interval = 50;  // delay time in ms

int PSTBY = 1000;
int P1L = 1006;
int P1H = 1019;

int SW1State = 0;  // current state of the button

void setup() {

  Serial.begin(115200);
  pinMode(MSpin, INPUT_PULLUP);
  pinMode(SW1, OUTPUT);
  digitalWrite(SW1, LOW);
  delay(500);
  digitalWrite(SW1, HIGH);

  attachInterrupt(digitalPinToInterrupt(MSpin), PulseTimerMS, CHANGE);
}

void PulseTimerMS() {
  CurrentTimeMS = micros();
  if (CurrentTimeMS > StartTimeMS) {
    PulsesMS = CurrentTimeMS - StartTimeMS;
    StartTimeMS = CurrentTimeMS;
  }
}

void loop() {
  //only save pulse lengths that are less than 2000 microseconds
  if (PulsesMS < 2000) {
    PulseWidthMS = PulsesMS;

    Serial.println(PulseWidthMS);
  }
  //noInterrupts();
  unsigned long currentMillis = millis();  // delay variable

  //if (currentMillis - previousMillis >= interval) {  // check if the current time minus the previous time is less or equal to the value on line 12
    //previousMillis = currentMillis;                  // if previous line is true, wait for amount of ms set on line 12

    //if (PulsesMS < PSTBY)
      if ((PulsesMS > P1L) && (PulsesMS < P1H))
                  digitalWrite(SW1, LOW);
        else digitalWrite(SW1, HIGH);
  //}
  //interrupts();
}
5

i changed the code, it toggles.
Now i have figure out how to implement a debounce
this the current code;

#define MSpin 2
#define SW1 4

volatile long StartTimeMS = 0;
volatile long CurrentTimeMS = 0;
volatile long PulsesMS = 0;
int PulseWidthMS = 0;

unsigned long previousMillis = 0;  // delay variable

const long interval = 50;  // delay time in ms

int PSTBY = 1000;
int P1L = 1006;
int P1H = 1019;

int SW1State = 0;  // current state of the button

void setup() {

  Serial.begin(115200);
  pinMode(MSpin, INPUT_PULLUP);
  pinMode(SW1, OUTPUT);
  digitalWrite(SW1, LOW);
  delay(500);
  digitalWrite(SW1, HIGH);

  attachInterrupt(digitalPinToInterrupt(MSpin), PulseTimerMS, CHANGE);
}

void PulseTimerMS() {
  CurrentTimeMS = micros();
  if (CurrentTimeMS > StartTimeMS) {
    PulsesMS = CurrentTimeMS - StartTimeMS;
    StartTimeMS = CurrentTimeMS;
  }
}

void loop() {
  //only save pulse lengths that are less than 2000 microseconds
  if (PulsesMS < 2000) {
    PulseWidthMS = PulsesMS;

    Serial.println(PulseWidthMS);
  }
  noInterrupts();
  unsigned long currentMillis = millis();  // delay variable

  if (currentMillis - previousMillis >= interval) {  // check if the current time minus the previous time is less or equal to the value on line 12
    previousMillis = currentMillis;                  // if previous line is true, wait for amount of ms set on line 12

    //if (PulsesMS < PSTBY)
    if ((PulsesMS > P1L) && (PulsesMS < P1H))
      digitalWrite(4, !digitalRead(4));
  }
  interrupts();
}
6

Yes.

The pulses keep coming, so every interval it satisfies the condition for toggling.

You must do something like

  if a valid pulse is within range
    if toggling is OK
      toggle and
      mark toggling as not OK


  if a valid pulse is not in the range
    mark toggling as OK

The toggle mark can just be a boolean variable

bool toggleOK = true;

and to keep it from happening again before the pukse is without the range,

    toggleOK = false;

This is state change (or edge) detection, and you will be toggling based on the pulse becoming valid rather than being valid.

There's a state change detection example in the IDE and a decent enough article here

https://docs.arduino.cc/built-in-examples/digital/StateChangeDetection/

Your signal in range vs. not in range is precisely analogous to a pushbutton pressed vs. not pressed.

a7

1 Like
8

Yes. You still have to go away and review the syntax of the if/else statement.

This

      if (toggleOK == true)
        ;
      digitalWrite(4, !digitalRead(4));

is precisely identical to

      digitalWrite(4, !digitalRead(4));

That semicolon is the statement being guarded by the if expression.

The toggle is escuted unconditionally, that is to say it always happens.

And this section should only reset the ability to toggle

  if (PulsesMS < PSTBY) {
    toggleOK = true;
  } 

// else (toggleOK = false);     no! toggling is what sets it to false!

I'm at the beach, so I still can't run your code or test my fixes.

So you have grasped the idea, which is sound, but have not yet implemented it. I think I've pointed out the defects.

I don't know if your if/else issues would in some case rise to the level of meriting a warning from the compiler. But in any case…

Go to the IDE preferences and dial up all warnings and verbosity, then be sure to read the red ink that some of your syntax mistakes might invoke.

a7

2 Likes
9

I'm curious why you have this in the first place. Whenever I have read in RC pulses, I just acted on what was received based on some threshold.
In other words, why pass PulseWidthMS = PulsesMS; at all if later on you're acting on PulsesMS; but debugging with this:

Serial.println(PulseWidthMS);

Other thought and I admit wrapping my head around using millis() (or micros()) sometimes makes my brain hurt even though it is a simple enough concept, but my question to you is this:

void PulseTimerMS() {
  CurrentTimeMS = micros();
  if (CurrentTimeMS > StartTimeMS) {
    PulsesMS = CurrentTimeMS - StartTimeMS;
    StartTimeMS = CurrentTimeMS;
  }
}

Can you please describe what this is doing in your code?

I don't see pulseIn used anywhere in your sketch to read in RC signals. That's what I use and it works. Have you considered using this?
https://www.arduino.cc/reference/en/language/functions/advanced-io/pulsein/

10

It's timing the length of pulses. Or it's timing the gap between pulses. It doesn't know which of those 2 things it is timing, could be either. @alto777 already pointed this out.

1 Like
11

Well I'm not as good at this as @alto777 , am I? :wink:

I must have missed that post.

12

Here it is:

Chances are it is measuring the lengths of pulses. But there's a ~10% chance it is measuring the gap between them. The interrupt routine doesn't actually check the signal to see if it's HIGH or LOW.

1 Like
13

what about something like this?

const int sw1 = 4; // left switch(?) 1 on pin 4
const int rx1 = 5;  // edit changed from 4 to say, 5 to reflect a channel in
unsigned long val;

void setup() {
  Serial.begin(115200);
  pinMode(rx1, INPUT);
  pinMode(sw1, OUTPUT);
  digitalWrite(sw1, LOW); // let's start LOW
}

void loop() {
  val = pulseIn(rx1, 20000);
  if (val <= 999) {
    digitalWrite(SW1, LOW);
  }
  else if (val >= 1000) {
    digitalWrite(SW1, HIGH);
  }
  Serial.println(val); //print to the serial port
}
14

Should work, yes.

(Although the else-if condition is unnecessary: if the pulse length isn't <= 999 it must be >= 1000.)

One possible "problem" with pulseIn() is that it is blocking code. It could waste up to 2ms of processing time every 20ms. In this case, I don't see it as a problem at all. I can't see that there's anything that the Arduino could be usefully doing during that "wasted" time.

1 Like
15

I expect some excitement no matter the approach when @rolf102 goes forward with

when i get this working i will add 17 more switch functions and use the arduino as a 18 channel multiswitch

At that point, it may make more sense to hunt down a library that can receive and decide the PPM, SBUS or IBUS signal that the receiver can output.

I don't use pulseIn(). I don't know if it would be hard to use it for decoding PPM, and it certainly would not be usable for SBUS or IBUS.

The multiple channels of PWM a receiver might offer could be fed to multiple inputs and each handled by pulseIn(), tests would have to be made to see how the blocking nature of the call affected such processing.

Of course it depends on what else is going on. Slow responses may not matter if it is not a car or helicopter being controlled.

a7

1 Like
16

Then the blocking nature of pulseIn() could potentially become a problem. Some pulses would inevitably be missed. Maybe 2 out of 3 would be missed. It's not clear if that would be a serious problem in this project or not.

But if interrupts were used, then no problem... except that most arduino don't have 18 external interrupt pins. Pin-change interrupts could be used I guess but they are a little more complex to implement.

I don't know as much about advanced RC data protocols, but it sounds like @alto777 does, so that could be a much more sensible approach than sampling 18 different PWM signals.

17

I think you can add an additional parameter to val = pulseIn(rx1, 20000); so it does this:

val = pulseIn(rx1, HIGH, 20000);

and then back to what you were saying, edge detection to count the spaces in between serially encoded signals? I'm not sure but found this:
https://rcarduino.blogspot.com/2012/11/how-to-read-rc-receiver-ppm-stream.html

18

It's working all 18 switches, thanks a lot!! @alto777.
Ill use it in a model tug boat, the perceived response is adequate.
I might try implementing SBUS and/or IBUS in the future when my programming ability has improved beyond where it is now.
I chose this method over using pulsein() for the very reason that it has a similar effect on the program as the basic delay() function.
i used THIS blog post as reference.

19

Please post the code you ended up with.

Also, what transmitter and receiver radio set are you using? And what kind of Arduino board?

a7

20

i use a Frsky Horus X10S express with a Frsky R4 receiver.
the Arduino is a Nano Every

21

OK THX.

It looks like you are only grabbing the control pulse from one channel, and the 18 actions are all ranges within what that channel can produce.

I am curious about what you are manipulating at the transmitter to control 18 toggles.

Can you reliably toggle #7 and #13, for example, without inadvertently toggling any of 8 through including 12?

I don't argue with success, but once you get a bit further along, use this sketch, which works, as the basis for version 2, which will benefit hugely from your knowledge of arrays, functions and loops and a few others of the next things you'll be learning.

I have a Nano Every. For many things they work perfectly well and no special fuss is required. As luck has it I am up to a few things that are different enough that the Every is waiting instead for the perfect project to build it into and forget about.

a7

1 Like