would like some feedback if possible on code.

hello,

i recently put together some code to control a toy helicopter using a ping sensor and some flex sensors attacked to gloves.

i did get the code to compile and for technically it does work but the latency makes the helicopter almost impossible to actually control. i will post my code below and hope someone might have some suggestions perhaps on how to lower the latency or at least smooth out my code, thanks in advance.

#include <SPI.h>
#include <NewPing.h>
#define TRIGGER_PIN 2
#define ECHO_PIN 4
#define MAX_DISTANCE 30
int slave = 10;
int flexpin=A0;
int flexpin1=A1;
int flexpin2=A2;
int flexpin3=A3;
int flex[20];
int flex1[20];
int flex2[20];
int flex3[20];
int flexsum=0;
int flexsum1=0;
int flexsum2=0;
int flexsum3=0;
int val=0;
int val1=0;
int val2=0;
int val3=0;
int val4=0;
int cm;
NewPing sonar(TRIGGER_PIN, ECHO_PIN, MAX_DISTANCE);
void setup()
{
pinMode(slave,OUTPUT);
SPI.begin();
Serial.begin(9600);
}
void setpot (int reg, int level)
{
digitalWrite(slave,LOW);
SPI.transfer(reg);
SPI.transfer(level);
digitalWrite(slave,HIGH);
}

void loop()

{
{
for(int x=0; x<20; x++)
{
flex[x]=analogRead(flexpin);
flex1[x]=analogRead(flexpin1);
flex2[x]=analogRead(flexpin2);
flex3[x]=analogRead(flexpin3);
flexsum=flexsum+analogRead(flexpin);
flexsum1=flexsum1+analogRead(flexpin1);
flexsum2=flexsum2+analogRead(flexpin2);
flexsum3=flexsum3+analogRead(flexpin3);

delayMicroseconds(14);
}
flexsum=flexsum/20;
flexsum1=flexsum1/20;
flexsum2=flexsum2/20;
flexsum3=flexsum3/20;

{
Serial.println(flexsum);
Serial.println(flexsum1);
Serial.println(flexsum2);
Serial.println(flexsum3);
delay(500);
}

{
delay(40);
unsigned int uS = sonar.ping();
int cm =(uS / US_ROUNDTRIP_CM);
Serial.print("Ping: ");
Serial.print(uS / US_ROUNDTRIP_CM);
Serial.println("cm");

{
int val = cm;
val=map(val,5,25,0,200);
val=constrain(val,5,200);
{
setpot(0,val);
}
}
int val1=flexsum;
if (val1 > 630)
{
int val1=map(val1,730,830,62,127);
{
setpot(1,val1);
}
}
else
{
int val2=flexsum1;
val2=map(val2,740,860,61,10);
{
setpot(1,val2);
}
}
int val3=flexsum3;
if (val3 > 570)
{
int val3=map(val3,570,770,62,127);
{
setpot(2,val3);
}
}
else
{
int val4=flexsum2;
val4=map(val4,630,820,61,10);
{
setpot(2,val4);
}
}
}
}
}

long mic2sec(long microseconds)
{
return microseconds / 29 / 2;
}

this is my first attempt at writing code for arduino and am a novice in general thus any advice is greatly appreciated.

delay(500);

Latency, you say?

(Do you see why we ask you to use code tags?)

AWOL:

delay(500);

Latency, you say?

(Do you see why we ask you to use code tags?)

yea, sorry i didn't use any tags (//?) because i didn't think i needed to know what it did but you're right i should have added some.

about the delay, i was under the impression that, that delay would only apply to the "print" and was not aware it would actually influence the code. i will give that a shot and if indeed it was that simple.. please allow me a /face palm lol

Your use of braces is confusing. Please re-post with code tags if you want further help.

I don't see the point of the second ones here, for example:

void loop()

{
  {

davngr, Code Tags are what makes your code look like this:

    for(int x=0; x<20; x++)
    {
      flex[x] = analogRead(flexpin);
      flex1[x] = analogRead(flexpin1);
      flex2[x] = analogRead(flexpin2);
      flex3[x] = analogRead(flexpin3);
...

instead of this:

for(int x=0; x<20; x++)
{
flex[x] = analogRead(flexpin);
flex1[x] = analogRead(flexpin1);
flex2[x] = analogRead(flexpin2);
flex3[x] = analogRead(flexpin3);
...

You did Quote tags. If you actually look at what your original post looks like, you'll see the problem.

This is your code without the unnecessary extra braces.
Does it still do what you think it does?

#include <SPI.h>
#include <NewPing.h>

#define TRIGGER_PIN      2
#define ECHO_PIN         4
#define MAX_DISTANCE    30

int slave       = 10;
int flexpin     = A0;
int flexpin1    = A1;
int flexpin2    = A2;
int flexpin3    = A3;
int flex[20];
int flex1[20];
int flex2[20];
int flex3[20];
int flexsum     = 0;
int flexsum1    = 0;
int flexsum2    = 0;
int flexsum3    = 0;
int val         = 0;
int val1        = 0;
int val2        = 0;
int val3        = 0;
int val4        = 0;
int cm;

NewPing sonar(TRIGGER_PIN, ECHO_PIN, MAX_DISTANCE);

void setup()
{
    Serial.begin(9600);

    SPI.begin();

    pinMode(slave, OUTPUT);
}
void setpot(int reg, int level)
{
    digitalWrite(slave, LOW);

        SPI.transfer(reg);
        SPI.transfer(level);

    digitalWrite(slave, HIGH);
}

void loop()
{
    for ( int x = 0; x < 20; x++ )
    {
        flex        = analogRead(flexpin);
        flex1       = analogRead(flexpin1);
        flex2       = analogRead(flexpin2);
        flex3       = analogRead(flexpin3);

        flexsum     = flexsum + analogRead(flexpin);
        flexsum1    = flexsum1 + analogRead(flexpin1);
        flexsum2    = flexsum2 + analogRead(flexpin2);
        flexsum3    = flexsum3 + analogRead(flexpin3);

        delayMicroseconds(14);
    }

    flexsum = flexsum / 20;
    flexsum1 = flexsum1 / 20;
    flexsum2 = flexsum2 / 20;
    flexsum3 = flexsum3 / 20;

    // ---

    Serial.println(flexsum);
    Serial.println(flexsum1);
    Serial.println(flexsum2);
    Serial.println(flexsum3);

    delay(500);

    // ---

    delay(40);

    unsigned int uS = sonar.ping();
    int cm  = (uS / US_ROUNDTRIP_CM);
    Serial.print("Ping: ");
    Serial.print(uS / US_ROUNDTRIP_CM);
    Serial.println("cm");


    int val = cm;
    val     = map(val, 5, 25, 0, 200);
    val     = constrain(val, 5, 200);
    setpot(0, val);

    int val1 = flexsum;
    if ( val1 > 630 )
    {
        int val1    = map(val1, 730, 830, 62, 127);
        setpot(1, val1);
    }
    else
    {
        int val2    = flexsum1;
        val2        = map(val2, 740, 860, 61, 10);
        setpot(1, val2);
    }

    int val3 = flexsum3;
    if ( val3 > 570 )
    {
        int val3    = map(val3, 570, 770, 62, 127);
        setpot(2, val3);
    }
    else
    {
        int val4    = flexsum2;
        val4        = map(val4, 630, 820, 61, 10);
        setpot(2, val4);
    }
}

long mic2sec(long microseconds)
{
    return microseconds / 29 / 2;
}

first off, thanks for the reply and apologies for lack of tags.

the extra brackets were added so that the code would compile. previously my code was much smaller and arranged in different successions but could not get the code to compile for the life of me and thus this is waht i was left with after hours of brainstorming. sadly i'm an absolute novice and frankly there was no absolute framework in the internet for what i was trying to accomplish.

i will try to clean up my code and add tags this weekend when i get some time off work and am able to actually set my helicopter back up for testing.

TanHadron:
davngr, Code Tags are what makes your code look like this:

    for(int x=0; x<20; x++)

{
      flex[x] = analogRead(flexpin);
      flex1[x] = analogRead(flexpin1);
      flex2[x] = analogRead(flexpin2);
      flex3[x] = analogRead(flexpin3);
...




instead of this:


> for(int x=0; x<20; x++)
> {
> flex[x] = analogRead(flexpin);
> flex1[x] = analogRead(flexpin1);
> flex2[x] = analogRead(flexpin2);
> flex3[x] = analogRead(flexpin3);
> ...



You did Quote tags. If you actually look at what your original post looks like, you'll see the problem.

yea, i'm not sure why it added those extra indents to my code as the original code did not have them. quite odd

lloyddean:
This is your code without the unnecessary extra braces.
Does it still do what you think it does?

#include <SPI.h>

#include <NewPing.h>

#define TRIGGER_PIN      2
#define ECHO_PIN         4
#define MAX_DISTANCE    30

int slave       = 10;
int flexpin     = A0;
int flexpin1    = A1;
int flexpin2    = A2;
int flexpin3    = A3;
int flex[20];
int flex1[20];
int flex2[20];
int flex3[20];
int flexsum     = 0;
int flexsum1    = 0;
int flexsum2    = 0;
int flexsum3    = 0;
int val         = 0;
int val1        = 0;
int val2        = 0;
int val3        = 0;
int val4        = 0;
int cm;

NewPing sonar(TRIGGER_PIN, ECHO_PIN, MAX_DISTANCE);

void setup()
{
    Serial.begin(9600);

SPI.begin();

pinMode(slave, OUTPUT);
}
void setpot(int reg, int level)
{
    digitalWrite(slave, LOW);

SPI.transfer(reg);
        SPI.transfer(level);

digitalWrite(slave, HIGH);
}

void loop()
{
    for ( int x = 0; x < 20; x++ )
    {
        flex        = analogRead(flexpin);
        flex1       = analogRead(flexpin1);
        flex2       = analogRead(flexpin2);
        flex3       = analogRead(flexpin3);

flexsum     = flexsum + analogRead(flexpin);
        flexsum1    = flexsum1 + analogRead(flexpin1);
        flexsum2    = flexsum2 + analogRead(flexpin2);
        flexsum3    = flexsum3 + analogRead(flexpin3);

delayMicroseconds(14);
    }

flexsum = flexsum / 20;
    flexsum1 = flexsum1 / 20;
    flexsum2 = flexsum2 / 20;
    flexsum3 = flexsum3 / 20;

// ---

Serial.println(flexsum);
    Serial.println(flexsum1);
    Serial.println(flexsum2);
    Serial.println(flexsum3);

delay(500);

// ---

delay(40);

unsigned int uS = sonar.ping();
    int cm  = (uS / US_ROUNDTRIP_CM);
    Serial.print("Ping: ");
    Serial.print(uS / US_ROUNDTRIP_CM);
    Serial.println("cm");

int val = cm;
    val     = map(val, 5, 25, 0, 200);
    val     = constrain(val, 5, 200);
    setpot(0, val);

int val1 = flexsum;
    if ( val1 > 630 )
    {
        int val1    = map(val1, 730, 830, 62, 127);
        setpot(1, val1);
    }
    else
    {
        int val2    = flexsum1;
        val2        = map(val2, 740, 860, 61, 10);
        setpot(1, val2);
    }

int val3 = flexsum3;
    if ( val3 > 570 )
    {
        int val3    = map(val3, 570, 770, 62, 127);
        setpot(2, val3);
    }
    else
    {
        int val4    = flexsum2;
        val4        = map(val4, 630, 820, 61, 10);
        setpot(2, val4);
    }
}

long mic2sec(long microseconds)
{
    return microseconds / 29 / 2;
}

thank you very much for taking the time to clean up my code and making it easier to read on the forum but sadly it does not compile in this form. this weekend i will try to massage your code a bit and see what part of my original code is not working with this cleaner format.

        flex        = analogRead(flexpin);
        flex1       = analogRead(flexpin1);
        flex2       = analogRead(flexpin2);
        flex3       = analogRead(flexpin3);

        flexsum     = flexsum + analogRead(flexpin);
        flexsum1    = flexsum1 + analogRead(flexpin1);
        flexsum2    = flexsum2 + analogRead(flexpin2);
        flexsum3    = flexsum3 + analogRead(flexpin3);

The first 4 lines need [ x ] added to them. But, what it the purpose of saving the data in the arrays, when you never use the data in the arrays? The last 4 lines do all the real work.

Exactly. That code could just as well be written:

void loop()
{
    flexsum = 0;
    flexsum1 = 0;
    flexsum2 = 0;
    flexsum3 = 0;
    
    for ( int x = 0; x < 20; x++ )
      {
      flexsum    += analogRead(flexpin);
      flexsum1   += analogRead(flexpin1);
      flexsum2   += analogRead(flexpin2);
      flexsum3   += analogRead(flexpin3);
      delayMicroseconds(14);
      }

No arrays necessary. However the code could be made neater by having an array for each pin, like this:

const byte NUMBER_OF_PINS = 4;
const byte flexpins [NUMBER_OF_PINS] = { A0, A1, A2, A3 };
int flexsums [NUMBER_OF_PINS];

...

void loop()
{
    for (int i = 0; i < NUMBER_OF_PINS; i++)
      flexsums [i] = 0;
    
    for ( int x = 0; x < 20; x++ )
      {
      for (int i = 0; i < NUMBER_OF_PINS; i++)
        flexsums [i] += analogRead (flexpins [i]);
      delayMicroseconds(14);
      }

    for (int i = 0; i < NUMBER_OF_PINS; i++)
      flexsums [i] /= 20;

    // ---

    for (int i = 0; i < NUMBER_OF_PINS; i++)
      Serial.println(flexsums [i]);

    delay(500);

Instead of repeating everything for pins 1, 2, 3 and 4, use a small loop. The code further down would need slight reworking.

thanks everyone for your post and suggestions.

i have put my project back together and revised my code but i still can't get reliable control of my helicopter so i'm just gona chalk this up to the digipot not being able to keep up with the incoming data(probably code related, a bit ambitious on my part i suppose). it does work in a limited fashion but when i try to use all the controls at once (as would be needed for flight) the commands are tardy or flat out lost.
i did learn a few tricks on this thread so it was not a total loss, thanks again.

I suggest you post your revised code if you want more help.