Go Down

### Topic: Apprentice coder (Read 6935 times)previous topic - next topic

#### Pedro147

#30
##### Oct 27, 2012, 12:31 amLast Edit: Oct 27, 2012, 04:39 am by Pedro147 Reason: 1
AWOL so as I understand it, you are suggesting that I
1.   use the array that I have declared in setup. How do I do this please.
2.   set the array to data type byte. So do I just change it from int to byte where I have declared it.
3.   define array by -

Code: [Select]
`#define N_ELEMENTS(array) (sizeof(array)/sizeof(array[0]))`.

So do I just place this definition at the start of the code and then somehow use "(sizeof(array)/sizeof(array[0]))" in the for loops where I was using int i
I apologise for asking so many questions but with all the suggestions and advice that awaited me when I arose this morning, I dont't know how to procede

In regard to PeterH's observation

Code: [Select]
`for(int i=0; i<9; i++)`

You're processing elements 0 .. 8 of an array that contains elements 0 .. 7.

So I also changed i<9 to i<8

Also Crossroads - yes I did
for (whatever){
anding operation;
}
didn't I ?

http://www.pedroduino.com

#### PeterH

#31
##### Oct 27, 2012, 02:10 am

So I also changed i<9 to i<8

That would fix the problem, but leaves you using an approach which makes this sort of problem likely.

You have a magic number ( 8 ) in several places in your code and the code will go wrong if they do not all match. Better to define the value as a constant and use it in all the places that need it.
Code: [Select]
`const int PIN_COUNT = 8;int pins [PIN_COUNT] = { 3, 4, 5, 6, 7, 8, 9, 10 };...for(int i=0; i<PIN_COUNT; i++)`

Even better is to have the code work the number out for itself, avoiding the chance that you used the wrong constant.

Code: [Select]
`#define N_ELEMENTS(array) (sizeof(array)/sizeof(array[0]))...int pins [] = { 3, 4, 5, 6, 7, 8, 9, 10 };...for(int i=0; i<N_ELEMENTS(pins); i++)`
I only provide help via the forum - please do not contact me for private consultancy.

#### Pedro147

#32
##### Oct 27, 2012, 03:29 amLast Edit: Oct 27, 2012, 04:14 am by Pedro147 Reason: 1
Thanks for that suggestion PeterH. So just to clarify if I may, I changed the code  and it still works the same [phew ]
Code: [Select]
` int timer = 100; #define N_ELEMENTS(array) (sizeof(array)/sizeof(array[0]))int pins [ ] = { 3, 4, 5, 6, 7, 8, 9, 10 };char led = B10000001;void setup () {  Serial.begin(9600); {    for(int i = 3; i < 11; i++)      pinMode(i, OUTPUT);  }}void loop (){  int mask = 1;  for(int i=0; i<N_ELEMENTS(pins); i++)  {   {      if((mask & led) == 0)        digitalWrite(pins[i], LOW);      else digitalWrite(pins[i], HIGH);      mask = mask << 1;      //  Serial.print(i);      //   delay(1000);   }  }}  `
Is that all you suggest I do. Does the rest of the code seem ok, and thanks again for your help there is so much to learn
Pedro.
http://www.pedroduino.com

#### PeterH

#33
##### Oct 27, 2012, 03:46 am

Is that all you suggest I do. Does the rest of the code seem ok, and thanks again for your help there is so much to learn

I suggest you also change:
Code: [Select]
`for(int i = 3; i < 11; i++)      pinMode(i, OUTPUT);`

to:
Code: [Select]
`for(int i=0; i<N_ELEMENTS(pins); i++){        pinMode(pins[i], OUTPUT); }`

Also, get into the habit of using a code block enclosed by { } after every control expression (if, else, for, while, do) even when you only have a single statement in it. It makes it much less likely that you will accidentally end up with a control structure that looks right but is actually wrong.

I also recommend that you put each { and } on a separate line with matching pairs indented by the same amount, and the contents indented one extra level. It makes it much easier to visually scan the structure of your code to confirm it matches your intention.
I only provide help via the forum - please do not contact me for private consultancy.

#### Pedro147

#34
##### Oct 27, 2012, 04:21 am
PeterH Thanks again for your help and suggestions and I will try and keep the code a bit tidier
http://www.pedroduino.com

#### lloyddean

#35
##### Oct 27, 2012, 05:54 amLast Edit: Oct 27, 2012, 06:59 am by lloyddean Reason: 1

The problem being everyone has their own  idea of what is readable!

Code: [Select]
`#define N_ELEMENTS(ARRAY)   (sizeof(ARRAY)/sizeof(ARRAY[0]))const uint8_t   pinLED_0    =  3;const uint8_t   pinLED_1    =  4;const uint8_t   pinLED_2    =  5;const uint8_t   pinLED_3    =  6;const uint8_t   pinLED_4    =  7;const uint8_t   pinLED_5    =  8;const uint8_t   pinLED_6    =  9;const uint8_t   pinLED_7    = 10;const uint8_t   pinsLEDS[]  = { pinLED_0, pinLED_1, pinLED_2, pinLED_3, pinLED_4, pinLED_5, pinLED_6, pinLED_7 };const uint8_t   LED_OFF     = LOW;const uint8_t   LED_ON      = HIGH;char            flagsLEDS   = B10000001;void setup () {//  Serial.begin(9600);    for ( int i = 0; i < N_ELEMENTS(pinsLEDS); i++ )    {        pinMode(pinsLEDS[i], OUTPUT);    }}void loop (){    for ( int mask = 0b00000001, i = 0; i < N_ELEMENTS(pinsLEDS); mask <<= 1, i++ )    {        digitalWrite(pinsLEDS[i], ((mask & flagsLEDS) ? LED_ON : LED_OFF));//      Serial.print(i);//      delay(1000);    }}`

#### Pedro147

#36
##### Oct 27, 2012, 08:40 am

The problem being everyone has their own  idea of what is readable!

Too true lloydean. Thank you for your take on my learning project and there is no denying that yours is one fine, concise piece of code. It certainly gives me another way of looking at things,     Pedro.
http://www.pedroduino.com

#### Pedro147

#37
##### Oct 28, 2012, 01:52 am
lloyddean, a few questions if I may. I was looking over that code that you posted for me and I noticed that it only used 960 bytes as opposed to  the 2,212 bytes used by the latest version of the code I was modifying (with much help from forum members) and I must say I like "high performance software"    and was wondering why this code is so efficient.

Also I find the fact that you declared so many constants at the start of the code interesting, and does this contribute to economical coding. In the line of code -

Code: [Select]
` digitalWrite(pinsLEDS[i], ((mask & flagsLEDS) ? LED_ON : LED_OFF));`

I could not find any reference to the ? symbol in any literature about Arduino coding. I see that uint8_t can be replaced by byte in this application. Thanks again for showing me this piece of code it certainly has given me something (else) to think about Pedro.

http://www.pedroduino.com

#### PeterH

#38
##### Oct 28, 2012, 02:38 am

I could not find any reference to the ? symbol in any literature about Arduino coding.

The Arduino programming language is essentially C++ with a little bit of tweaking. The conditional operator ? is part of the standard C++ language (and several other similar languages, too):

http://en.wikipedia.org/wiki/%3F:

Although the Arduino reference pages describe some aspects of 'C' and C++, they don't cover everything and don't go into much detail. It's worth getting a good book on C++ or read through some online tutorials to get a better feel for what the language can do. Just remember that most examples are designed to run on a computer will an operating system that provides features not provided by the Arduino runtime environment, so when you see C++ code that uses runtime libraries and interacts with the operating system make a mental note that on Arduino most of those features are simply not available.
I only provide help via the forum - please do not contact me for private consultancy.

#### Pedro147

#39
##### Oct 28, 2012, 02:56 am
Thanks PeterH ,
yes I have read a little about C++ and saw many things that were familiar from my limited  Arduino coding experience. I believe that there are, as they say "many things under the hood" when using the Arduino IDE, and that coding in C type syntax presented to the Arduino IDE in a way that it can interpret, is much more efficient code than the traditional Arduino coding structure. But in saying that I suppose that the whole Arduino experience is designed to give more people access to what is termed physical computing without getting too bogged down in full on learning C or C++ programming. Heck I have enough trouble with the parred down Arduino version but as they say, where there's a will there's a way
Thanks again for your time Pedro.
http://www.pedroduino.com

#### lloyddean

#40
##### Oct 28, 2012, 05:26 amLast Edit: Oct 28, 2012, 05:53 am by lloyddean Reason: 1
The main reason for the size difference is probably mine doesn't link in the 'Serial' code.

If you locate your Arduino header files, I'm on Mac OS where it is located in the Application bundle at '
/Applications/Mpide.app/Contents/Resources/Java/hardware/arduino/cores/arduino', and look thru the file 'wiring_digital.c'
you'll discover that 'pinMode', 'digitalWrite' and 'digitalRead' have the been declared as ...

Code: [Select]
`void pinMode(uint8_t pin, uint8_t mode);void digitalWrite(uint8_t pin, uint8_t val);int digitalRead(uint8_t pin);`

This shows that the parameters to all three function are of type 'uint8_t' not 'int' or 'byte'.

Code: [Select]
`    int pins [ ] = { 3, 4, 5, 6, 7, 8, 9, 10 };`

By declaring the array as type 'int' you're reserving space in RAM. Also  an 'int' on the Arduino boards (except the new Arduino Due) occupy 2 bytes of RAM versus 1 byte for 'uint8_t'.

By declaring 'pins[]' as 'uint8_t pins[]' we save 1 byte of RAM per array entry.

Since the values of pins usaully don't need to be changed at run time it should be declared as 'const uint8_t pins[]'.  Declared this way it can not be accidentally changed during the running of your sketch.  Another benefit is that they are now compile time constants and occupy no RAM at all.

As to ...

Code: [Select]
`const uint8_t   pinLED_0    =  3;const uint8_t   pinLED_1    =  4;const uint8_t   pinLED_2    =  5;const uint8_t   pinLED_3    =  6;const uint8_t   pinLED_4    =  7;const uint8_t   pinLED_5    =  8;const uint8_t   pinLED_6    =  9;const uint8_t   pinLED_7    = 10;`

Named constants occuppy no memory space but allow your code to use the name as a synonym for the assigned constant making for easier to read code.

By the way a couple of simple modification to both 'for' loops could further reduce the size of the code while also speeding execution time.

#### Pedro147

#41
##### Oct 28, 2012, 08:36 am
Thanks for that explanation lloyddean. So basically declaring constants as type uint8_t uses less memory than a byte or int declarations and named constants for pin output declaration uses no memory. I like that How might the for loops be modified to reduce code size and increase speed execution if I may ask. Thank you for the lesson in efficient concise coding 101
http://www.pedroduino.com

#### lloyddean

#42
##### Oct 29, 2012, 10:17 pm

How might the for loops be modified to reduce code size and increase speed execution if I may ask.

Tried to find the previous discussion on the subject several years back but I believe it is in the 'old' forums database.  I'll just post the modifications and let the discussion reemerge as a result.

Most processors are able to count down to zero faster and with less code than counting up, comparing the counter to a 'ceiling' value and deciding whether to loop again or not.

Code: [Select]
`#define N_ELEMENTS(ARRAY)   (sizeof(ARRAY)/sizeof(ARRAY[0]))const uint8_t   pinLED_0    =  3;const uint8_t   pinLED_1    =  4;const uint8_t   pinLED_2    =  5;const uint8_t   pinLED_3    =  6;const uint8_t   pinLED_4    =  7;const uint8_t   pinLED_5    =  8;const uint8_t   pinLED_6    =  9;const uint8_t   pinLED_7    = 10;const uint8_t   pinsLEDS[]  = { pinLED_0, pinLED_1, pinLED_2, pinLED_3, pinLED_4, pinLED_5, pinLED_6, pinLED_7 };const uint8_t   LED_OFF     = LOW;const uint8_t   LED_ON      = HIGH;char            flagsLEDS   = B10000001;void setup () {//  Serial.begin(9600);     for ( int i = N_ELEMENTS(pinsLEDS); i--; )    {        pinMode(pinsLEDS[i], OUTPUT);    }}void loop (){    for ( int mask = 0b10000000, i = N_ELEMENTS(pinsLEDS); i--; mask >>= 1 )    {        digitalWrite(pinsLEDS[i], ((mask & flagsLEDS) ? LED_ON : LED_OFF));//      Serial.print(i);//      delay(1000);    }}`

#### Pedro147

#43
##### Oct 30, 2012, 02:11 am
Thank you for that insight lloyddean. Who would have thought  Pedro.
http://www.pedroduino.com

#### Pedro147

#44
##### Oct 31, 2012, 09:25 am
I have been scratching my head (again  ) trying to work out how to modify the Knightrider type code to flash each LED sequentially x number of  times while looping through the array. That is for example, first LED flashes five times then second LED flashes five times etc through the loop and after it flashes LED eight five times it does the same routine back to the start. Could someone give me a few prompts as to what I am missing. Here is my basic starting code.
Code: [Select]
` int timer = 100;int led [] = { 3, 4, 5, 6, 7, 8, 9, 10 };int pinCount = 8; void setup () {   for(int i = 0; i < pinCount; i++)  pinMode(led[i], OUTPUT);}void loop (){  for(int i=0; i<pinCount; i++)  {     digitalWrite(led[i], HIGH);    delay (timer);    digitalWrite(led[i], LOW);  }   {     for(int i = 7; i>0; i--)     {        digitalWrite(led[i], HIGH);       delay(timer);       digitalWrite(led[i], LOW);      }  }}`

As an aside I would like to wish all our American forum members the best as they face this horrific natural disaster and hope that them and their family and friends are safe,
Pedro.

http://www.pedroduino.com

Go Up

Please enter a valid email to subscribe