Setting an random LED high

[Alex]

Does it? Lets test it. Say the rand funtion returns 16. What value will be stored in value and which led will light up?

[eliocor]

Does it? Lets test it. Say the rand funtion returns 16. What value will be stored in value and which led will light up?

Really simple:
16 = 00010000 in binary
ANDing the 0x07 mask to 16 results:
00010000 AND 00000111 = 00000000
so the result is 0

The 0x07 mask sets to 0 the most significant bits of your number:
00000xxx
Only the bit marked with 'x' will remain untouched.
The operation is the same as follows:

Value = rand() % 8 ;

but using the '&' operator (AND) is faster and (to me) more readable

[Alex]

Does it? Lets test it. Say the rand funtion returns 16. What value will be stored in value and which led will light up?

Really simple:
16 = 00010000 in binary
ANDing the 0x07 mask to 16 results:
00010000 AND 00000111 = 00000000
so the result is 0

The 0x07 mask sets to 0 the most significant bits of your number:
00000xxx
Only the bit marked with 'x' will remain untouched.
The operation is the same as follows:

Value = rand() % 8 ;

but using the '&' operator (AND) is faster and (to me) more readable

...and thats why no LED (or only the first LED) will light up for generated numbers that do not have any of the three LSBs set. Thats a lot of numbers.

[IanB]

...and thats why no LED (or only the first LED) will light up for generated numbers that do not have any of the three LSBs set. Thats a lot of numbers.

I don't get that. There are 8 LEDs and 8 values, 0 - 7.

0 => LED #1,
1 => LED #2,
2 => LED #3,
...,
7 => LED #8.

[Alex]

...and thats why no LED (or only the first LED) will light up for generated numbers that do not have any of the three LSBs set. Thats a lot of numbers.

I don't get that. There are 8 LEDs and 8 values, 0 - 7.

0 => LED #1,
1 => LED #2,
2 => LED #3,
...,
7 => LED #8.

If the OP just copies the number stored in 'value' to the LED port then only 3 of the LEDs will light up, LED 1-3. A binary to decimal decoder will have to be implemented. Thats one issue.

The other issue is that a portion of the dynamic range of the random number generated is being utilised. If the binary representation of the number generated by the rand function does not contain any 1 in the 3 LSBs then the masked value will be 0. In other words, you are not scaling the random number to fit 8 LEDs, you are truncating it.

[senso]

Yes, I never use the % in my micro-controllers code, only in pic32 that as hardware modulus and so its as fast as doing it or the &, but I was typing with one hand because I was freezing with cold, and I wanted to pass the idea to the OP, there are lots of options to choose from.
One way to get some random seeds is to read an analog channel, and you probably can make it more random if you read it 16 times and only store the first(bit 0) of each reading and then make a 16 bits value out of it.

[IanB]

If the OP just copies the number stored in 'value' to the LED port then only 3 of the LEDs will light up, LED 1-3. A binary to decimal decoder will have to be implemented. Thats one issue.

What "LED port"? What makes you think the OP is going to do something that won't work? You have constructed a straw man argument based on an imaginary hypothesis in order to knock it down. I think you should give the OP more credit, especially since this:

This workes like a charm!

I mean, if the OP has tested it and found it working, why do you think it only lights 3 LEDs?

The other issue is that a portion of the dynamic range of the random number generated is being utilised. If the binary representation of the number generated by the rand function does not contain any 1 in the 3 LSBs then the masked value will be 0. In other words, you are not scaling the random number to fit 8 LEDs, you are truncating it.

Here you do have a point. Taking the lowest few bits of a random number will reduce the randomness significantly. But for the purposes of this exercise it was accepted higher up the thread that perfection was not required. Nevertheless, it is as well to be aware of the limitations of the algorithm chosen.

[Alex]

Ian, we have to work with fragmented info here. Regarding ports, the op mentioned 8 leds with resistors. Thats a typical reply of a number of leds connected to an output port of the mcu. Pelle, how have you connected your leds? The connection method is important as it links with the code.

About the 3 leds. The op quoted a block of code that doesnt do bcd conversion. As you say the op says it works, so there is a conversion taking place at some point. That should be mentioned.Maybe if i had used past tense it would be even more clear what i meant, which is the scenario thay no conversion is used.

Thats all there is to the above points.

I am glad you understood what i meant about the range of the number. This piece of info is crucial if what you want is equal probability for all leds.

[IanB]

My interpretation is that the MCU has eight of its pins designated as outputs. Each pin is connected to an LED via a current limiting resistor. Set the output high to turn on the LED, set the output low to turn off the LED.

The above code was posted:

Code: [Select]

unsigned char Value=0, oldValue=0 ;

for (;;)
{
   Value = rand() & 0x07 ; // mask all most significant bits except the 3 LSB:
                           // 'Value' is between 0 and 7
   if (Value != OldValue)
      {
      OldValue = Value ;
      // light the 'nth' LED using the number (0-7) stored in 'Value'
      }
 }

It all depends what code lies behind "light the 'nth' LED using the number (0-7) stored in 'Value'". This comment tells me the intent is clearly understood, but we don't have sight of the actual code. Maybe there is a pair of functions like this:

Code: [Select]

unsigned char Value=0, oldValue=0 ;

for (;;)
{
   Value = rand() & 0x07 ; // mask all most significant bits except the 3 LSB:
                           // 'Value' is between 0 and 7
   if (Value != OldValue)
      {
      OldValue = Value ;
      clear_LED(OldValue);
      light_LED(Value);
      }
 }

I won't try to write these functions; they could be written using a switch() statement, or more simply by indexing the output pins off of Value. Something like "set_pin(base+value);" even. (Where set_pin(index) sets the state of the indexed output pin to high.)

[l4rtt-1]

I won't try to write these functions; they could be written using a switch() statement, or more simply by indexing the output pins off of Value. Something like "set_pin(base+value);" even. (Where set_pin(index) sets the state of the indexed output pin to high.)

Or just as simple as:

PORTX = 1 << Value;

[pelle.jansen]

Hi all
First I want to thank you all for your input!
I did get it working with the srand() reading an input from the analog channel which is floating. There is only one thing: I had to use 8(!) if statements to get it to work. It works but is very ugly of course. Every way I try to solve it it doesn't work. Some tips would me much appreciated!

Code: [Select]

/******************************************************
 * Pelle Jansen
 * 5 - 2 - 2012
 *
 * Sets a random LED high, LED can be cleared by pressing corresponding button.
 *
 * PIC16F877A
 *
 * X1: 8 MHZ, 2 22 pF capacitors to ground
 * Code is still ugly as shit, please bear with me.
 * ****************************************************
 */

#include <htc.h> // main PIC header file
#include <stdlib.h> // Reqired for randomize() and rand()
#include "delay.h" // for DelayUs()
#include "delay.c" // for some reason necessary for DelayMs()


/* External high speed 8 MHZ crystal oscillator, Watchdog timer disabled
 * brouwn out reset enabled, EEPROM protection disabled
 * write protect disabled, code protect disable, Low voltage programming disabled
 * power on timer enabled.
 */
__CONFIG(FOSC_HS & WDTE_OFF & BOREN_ON & CPD_OFF & WRT_OFF & CP_OFF & LVP_OFF & PWRTE_ON);

/*system*/
#define XTAL_FREQ 8 MHZ

/*functions*/
int rando(){
    int value;
    value = rand() & 0x07;
    return value;
}

/*main function*/
void main (void)
{
    TRISD=0x00; // PORTD as output
    TRISB=0xff; // PORTB as input

    ADCON1 = 0b00010000;
    ADCON0 = 0b00000001;
    DelayUs(1);
    GO_DONE = 1;
    while (GO_DONE)
        ;
    srand(ADRESH);

    int randval, oldval, out;
    randval=rando();

    while (1) // infinite loop
    {

      if (randval != oldval){
          PORTD = 1 << randval;

          if (RB0 == 0){
              out=0;
          }         
          else if (RB1 == 0){
              out=1;
          }
          else if (RB2 == 0){
              out=2;
          }
          else if (RB3 == 0){
              out=3;
          }
          else if (RB4 == 0){
              out=4;
          }
          else if (RB5 == 0){
              out=5;
          }
          else if (RB6 == 0){
              out=6;
          }
          else if (RB7 == 0){
              out=7;
          }

          if (out==randval){
              randval=rando();
              oldval=randval;
          }

          else{
              continue;
          }
         
      }
      else{
          randval=rando();
      }

    }
}
Analog inputs were a lot easier than I thought. A good read trough the datasheet gave me everything I needed.

[eliocor]

1) it is more than 15 years I'm not using PICs, so I hope I remember correctly the I/O port syntax...
2) I modified the code without trying to compile it, so maybe there are some errors in it
3) You never mentioned you was checking an input port (PORTB) and required to read its status: I hope to understand there can be a bit set to 0 and you have to identify it...
4) i removed your 'rando()' function because you need to call it only 1 time...

Code: [Select]

void main (void)
{
    TRISD=0x00; // PORTD as output
    TRISB=0xff; // PORTB as input

    ADCON1 = 0b00010000;
    ADCON0 = 0b00000001;
    DelayUs(1);
    GO_DONE = 1;
    while (GO_DONE)
        ;
    srand(ADRESH);

    unsigned char randval=0, oldval=0, out, i; // they are UNSIGNED CHAR!!!!
    randval=rando();

    while (1) // infinite loop
        {
      randval = rnd() & 0x07 ; // only values from 0 to 7
       
        if (randval != oldval)
            {
            // assign value to the 'old' variable
            oldval = randval ;
            // light the 'n'th led
            PORTD = 1 << randval ;
           
            // scan the 8 input ports
            // 'out' is set to 0xFF to avoid undefined cases
            for (out=0xFF, i=0 ; i<8 ; i++)
                {
                // check if input port 'i' is set to '0'
                // the loop will be exited at the first occurrence of '0' on PORTB
                // be careful! if every bit of PORTB is set to '1', 'out' value is UNDEFINED!!!
                if (!(PORTB & (0x01 << i)))
                    {
                    out = i ;    // set 'out' to number of port found to 0
                    break ;      // exit from the loop: I have found a bit set to 0
                    }
                }
            }
      }
}

[added some hours later]
Corrected the code to use Bitwise AND operator (&), instead of Logical AND operator (&&)

[pelle.jansen]

1) it is more than 15 years I'm not using PICs, so I hope I remember correctly the I/O port syntax...
2) I modified the code without trying to compile it, so maybe there are some errors in it
3) You never mentioned you was checking an input port (PORTB) and required to read its status: I hope to understand there can be a bit set to 0 and you have to identify it...
4) i removed your 'rando()' function because you need to call it only 1 time...

1. 15 years is a long time . What do you use now?
2. There were some errors but they were easy to correct.
3. I thought it would be relatively easy. Just reading a input and writing it in a variable sounds soo simple but it isn't.
4. Actually 2 times 

But... it doesnt work. Your for loop seems to always return 8 (all leds light if you assign it to PORTD). Here is the modified code:

Code: [Select]

/******************************************************
 * Pelle Jansen
 * 5 - 2 - 2012
 *
 * PIC16F877A
 *
 *Sets a random LED high, LED can be cleared by pressing corresponding button.
 *
 * X1: 8 MHZ, 2 22 pF capacitors to ground
 *
 * ****************************************************
 */

#include <htc.h> // main PIC header file
#include <stdlib.h> // Reqired for randomize() and rand()
//#include "delay.h" // for DelayUs()
#include "delay.c" // for some reason necessary for DelayMs()


/* External high speed 8 MHZ crystal oscillator, Watchdog timer disabled
 * brown out reset enabled, EEPROM protection disabled
 * write protect disabled, code protect disable, Low voltage programming disabled
 * power on timer enabled.
 */
__CONFIG(FOSC_HS & WDTE_OFF & BOREN_ON & CPD_OFF & WRT_OFF & CP_OFF & LVP_OFF & PWRTE_ON);

/*system*/
#define XTAL_FREQ 8 MHZ


/*main function*/
void main (void)
{
    TRISD=0x00; // PORTD as output
    TRISB=0xff; // PORTB as input

    ADCON1 = 0b00010000;
    ADCON0 = 0b00000001;
    DelayUs(5);
    GO_DONE = 1;
    while (GO_DONE)
        ;
    srand(ADRESH);

    unsigned char randval=0, oldval=0, out, i;
   
    randval = rand() & 0x07; //

    while (1) // infinite loop
    {

      if (randval != oldval){
          PORTD = 1 << randval;

          for (out=0xFF, i=0 ; i<8 ; i++){
            // check if input port 'i' is set to '0'
            // the loop will be exited at the first occurrence of '0' on PORTB
            // be careful! if every bit of PORTB is set to '1', 'out' value is UNDEFINED!!!
            if (!(PORTB && (0x01 << i)))
                {
                out = i;    // set 'out' to number of port found to 0
                break;      // exit from the loop: I have found a bit set to 0
                }
            }

          if (out==randval){
              oldval=randval;
              randval= rand() & 0x07;
             
          }
      }
     
      else
          continue; // is this necessary?

    }
}I am still pretty unfamiliar with the &, &&, | etc operators. That happens when you get spoiled by learning a high level language first 

[eliocor]

1) mostly Freescale and Arm (NXP/ST)

2)what is the use of your reading on PORTB: please can you explain it?

3) same for this piece of code:
if (out==randval){
  oldval=randval;
  randval= rand() & 0x07;
  }

It is really not clear (to me) what you want to do...

I have the slight impression there is something 'cheesy'....

[pelle.jansen]

It is stated in the comment on top of code.
 *Sets a random LED high, LED can be cleared by pressing corresponding button.
It is part of a little fun reaction time game.

[eliocor]

BTW, I wrote the wrong operator:
instead of '&&' (logical AND)
you have to put the '&' (bit AND):

if (!(PORTB & (0x01 << i)))
in such way it will exit when it will find the FIRST 0 (counting from B0 to B7).

Sorry for the inconvenience (it was written in a hurry)

[pelle.jansen]

BTW, I wrote the wrong operator:
instead of '&&' (logical AND)
you have to put the '&' (bit AND):

if (!(PORTB & (0x01 << i)))
in such way it will exit when it will find the FIRST 0 (counting from B0 to B7).

Sorry for the inconvenience (it was written in a hurry)

Yup, that did it 

No problem. It just proves that yore human Only thing was that I tried for like 15 minutes before thinking it won't work.
Can you give me some clarification about those operators? I am totally unfamiliar with them and my C tutorial doesn't cover it for some reason.

Thanks for your input!

[pelle.jansen]

Or not.
The PIC seems to freeze after a sort while   
What could cause this?

[eliocor]

No idea about the freezing, but I do not think it depends on the code...

regarding the Bitwise operations, please take a look at the following links:
http://en.wikipedia.org/wiki/Bitwise_operation
http://en.wikipedia.org/wiki/Bitwise_operations_in_C

[pelle.jansen]

Thanks, I will definitely read through that article!
Now up to solving that darn freeze problem

[pelle.jansen]

Hi all,
I rebooted my Mac, unplugged and replugged my PicKit 3 and TADA it worked . Some strange glitch in the compiler or MPLAB X I guess. Well for all of you interested, here is the final code. Fully annotated so everyone will understand what I am trying to do.

Code: [Select]

/*******************************************************************************
 * Pelle Jansen                                                                *
 * 10 - 2 - 2012                                                               *
 * V1.1                                                                        *
 * Free to distribute, use, modify, whatever you like ;)                       *
 *                                                                             *
 * Designed for PIC16F877A                                                     *
 *                                                                             *
 * Reaction time game                                                          *
 * Sets a random LED high, LED can be cleared by pressing corresponding button.*
 *                                                                             *
 * X1: 8 MHZ, 2 22 pF capacitors to ground                                     *
 * RA0: for random number. LEAVE FLOATING!                                     *
 * Entire PORTB: 8 switches to ground with external Pull up resistors          *
 * Entire PORTD: 8 LEDs with corresponding resistors                           *
 *                                                                             *
 ******************************************************************************/

#include <htc.h> // main PIC header file
#include <stdlib.h> // Reqired for randomize() and rand()
#include "delay.c" // for some reason necessary for DelayMs()

/* External high speed 8 MHZ crystal oscillator, Watchdog timer disabled
 * brouwn out reset enabled, EEPROM protection disabled
 * write protect disabled, code protect disable, Low voltage programming disabled
 * power on timer enabled.
 */
__CONFIG(FOSC_HS & WDTE_OFF & BOREN_ON & CPD_OFF & WRT_OFF & CP_OFF & LVP_OFF & PWRTE_ON); // config bits

/*Defenitions. Defines, global variables funcions etc*/
/*system*/
#define XTAL_FREQ 8 MHZ /*required for DelayUs() and DelayMs()*/
/*user*/
// add user defenitons
/*macro's*/
//add user macro's

/*main function*/
void main (void)
{
    /*Digital initilization*/
    TRISD=0x00; // PORTD as output
    TRISB=0xff; // PORTB as input

    /*Analog initilization*/
    ADCON1 = 0b00010000; // set analog config registers
    ADCON0 = 0b00000001;
    DelayUs(20); // wait for analog channel 0 to get a reading
    GO_DONE = 1;
    while (GO_DONE)// wait for reading to complete
        ;
    srand(ADRESH); // seed the random analog input value to the rand() function
    /*Local variable defenition*/
    unsigned char randval, oldval=0, out, i; // variable definiton
    /*initilization*/
    randval = rand() & 0x07; // get initial random number between 0 and 7
    /*Main infinite loop*/
    while (1)
    {
        if (randval != oldval){ // make sure the number is not the same as the last one
            PORTD = 1 << randval; // set the value on LEDs shiftet one to the left

          for (out=0xFF, i=0 ; i<8 ; i++){ // check each for 0 the loop will be exited at the first occurrence of '0' on PORTB
            // be careful! if every bit of PORTB is set to '1', 'out' value is UNDEFINED!!!
            if (!(PORTB & (0x01 << i))){
                    out = i;    // set 'out' to number of port found to 0
                    break;      // exit from the loop: I have found a bit set to 0
                }
          }
          if (out==randval){ // if the random number is equal to the number gotten from the buttons
              oldval=randval; // set the oldval variable to the current (old) random number
              randval= rand() & 0x07; // generate new random number between 0 and 7
        }
        else{ // if the random value equals to the old random value
            oldval=randval; // set the oldval variable to the current (old) random number
            randval= rand() & 0x07; // generate new random number between 0 and 7
        }
        }
    }

}

Thanks all for your support. Couldn't have done it without your support guys!