NerdKits - electronics education for a digital generation

You are not logged in. [log in]

NEW: Learning electronics? Ask your questions on the new Electronics Questions & Answers site hosted by CircuitLab.

Microcontroller Programming » Different result with different variable names

May 01, 2013
by scootergarrett
scootergarrett's Avatar

I’m getting different result when all I’m doing is changing a variable name ‘Done’ gives strange results but if I replace it in my program with ‘Done1’ it works as expected, also ‘LastSample’ doesn’t work. It only seems to give strange results when I include:

// start up the Analog to Digital Converter //
ADMUX = 0;
ADCSRA = (1<<ADEN) | (1<<ADPS2) | (1<<ADPS1) | (1<<ADPS0);
ADCSRA |= (1<<ADSC);

I’m not using those variables anywhere else in my program. Could there be something on in of the header that is causing a conflict? I search through them bud didn't find anything. I have a lot going on in the program there are two interrupts but if you run it then replace all ‘Done’ for ‘Done1’ it works differently. here is the code and the (#include "SDHC.h") from Fun with SDHC cards also it compiles slightly different screen shots from each load Done vs Done1 there is 10038 vs 10040 bytes written respectively. Any ideas, Thanks.

Raw crappy code:

// for NerdKits with ATmega328p

#define F_CPU 14745600

#include <stdio.h>
#include <math.h>
#include <avr/io.h>

#include <avr/interrupt.h>
#include <avr/pgmspace.h>
#include <inttypes.h>

#include "../libnerdkits/io_328p.h"
#include "../libnerdkits/delay.h"
//#include "../libnerdkits/uart.h"
#include "../libnerdkits/lcd.h"

#include "SDHC.h"

#define FirstDataSector 30304

volatile short k;
volatile uint32_t CurrentSample;
volatile uint32_t LastSample;
volatile char Done;                             // Problem with this

uint16_t adc_read(char port)
{
    ADMUX = port;
    ADCSRA |= (1<<ADSC);
    while(ADCSRA&(1<<ADSC));
    return (ADCL+(ADCH<<8));
}

int main()
{
    uint32_t CurrentSector;

    CurrentSector = FirstDataSector;
    k = 0;
    CurrentSample = 0;
    LastSample = 0;
    Done = 0;

    DDRC |= (1<<PC3);               // Make PC3 an output for testing
    PORTC  &= ~(1<<PC3);

    DDRB &= ~(1<<PB1);              // Set pin PB1 as input to signal start and stop
    PORTB |= (1<<PB1);              // Turn on PB1 pull up resistor

    delay_ms(100);

    // start up the Analog to Digital Converter //
    ADMUX = 0;
    ADCSRA = (1<<ADEN) | (1<<ADPS2) | (1<<ADPS1) | (1<<ADPS0);
    ADCSRA |= (1<<ADSC);

    // Interrupt setup stuff //
    TCCR1B |= (1<<CS12) | (1<<CS10) | (1<<WGM12);
    TIMSK1 |= (1<<OCIE1A);
    OCR1A = 143;                 //Interrupt timing

    // Start up the LCD //
    lcd_init();
    FILE lcd_stream = FDEV_SETUP_STREAM(lcd_putchar, 0, _FDEV_SETUP_WRITE);
    lcd_home();

    PCICR |= (1<<PCIE0);
    PCMSK0 |= (1<<PCINT1);
    sei();

    while(1)
    {
        if(Done)
        {
            cli();                      // Turn off interrupts
            break;
        }

      //  PORTC  ^= (1<<PC3);

        if(k*2>512)
        {
         //   LastSample = 0;
            k = 0;
            CurrentSector++;
        }

        lcd_home();
        fprintf_P(&lcd_stream, PSTR("Done: %i   k: %i"), Done, k);

        lcd_line_two();
        fprintf_P(&lcd_stream, PSTR("CurrentSample: %ld"), CurrentSample);

        lcd_line_three();
        fprintf_P(&lcd_stream, PSTR("LastSample: %ld"), LastSample);

        lcd_line_four();
        fprintf_P(&lcd_stream, PSTR("CurrentSector: %ld"), CurrentSector);

     //   delay_ms(100);
    }

    lcd_home();
    fprintf_P(&lcd_stream, PSTR("Done: %i   k: %i"), Done, k);

    lcd_line_two();
    fprintf_P(&lcd_stream, PSTR("CurrentSample: %ld"), CurrentSample);

    lcd_line_three();
    fprintf_P(&lcd_stream, PSTR("LastSample: %ld"), LastSample);

    lcd_line_four();
    fprintf_P(&lcd_stream, PSTR("CurrentSector: %ld"), CurrentSector);

    while(1);

    return 0;
}

ISR(TIMER1_COMPA_vect)
{
  //  *((uint16_t*)&buffer[k*2]) = adc_read(0);
    ++k;
    ++CurrentSample;

    return;
}

/// Last sample signal interrupt ///
ISR(PCINT0_vect)
{
    if(CurrentSample > 350)         // needs to get at least  > xxx samples before it will turn off
    {
        if(LastSample == 0)
        {
            Done = 1;
            LastSample = CurrentSample;
        }
    }

    return;
}

And "SDHC.h" header just for completeness

//sdhc.h
// Minimum SDHC commands
#define GO_IDLE_STATE       0
#define SEND_OP_COND        1
#define SEND_IF_COND        8
#define READ_SINGLE_BLOCK   17
#define WRITE_SINGLE_BLOCK  24
#define SD_SEND_OP_COND     41     //ACMD
#define APP_CMD             55
#define READ_OCR            58

#define SDHC_ERR_NONE        0
#define SDHC_ERR_TIMEOUT     1
#define SDHC_ERR_TIMEOUT_OP  2
#define SDHC_ERR_TIMEOUT_OCR 3
#define SDHC_ERR_NOT_SDHC    4
#define SDHC_ERR_R_TIMEOUT   5
#define SDHC_ERR_W_TIMEOUT   6

// Set SS as output - Set SS to high (disable)  low (enable)
#define SD_SS_INIT      DDRB |= 1<<PB2
#define SD_SS_ENABLE    PORTB &= ~(1<<PB2)
#define SD_SS_DISABLE   PORTB |= (1<<PB2)

#define BUFFER_SIZE 512

volatile uint8_t buffer[BUFFER_SIZE];

// spi.c
void spi_init(void);
uint8_t spi_tx(uint8_t data);
uint8_t spi_rx(void);

//sdhc.c
uint8_t sdhc_init(void);
uint8_t sdhc_send_comm(uint8_t cmd, uint32_t arg);
uint8_t sdhc_read_block(uint32_t startBlock);
uint8_t sdhc_write_block(uint32_t startBlock);

// spi.c
void spi_init(void) // Setup SPI: Master mode, MSB first, SCK phase low, SCK idle low, 1/64 Clk
{
  DDRB |= (1<<PB3) | (1<<PB5);  // Set MOSO, SCK as output
  DDRB &= ~(1<<PB4);            // Set MISO as input
  SPCR |= (1<<SPE) | (1<<MSTR) | (1<<SPR1);
  PORTB |= 1<<PB4;          // Set MISO pull-up
}

uint8_t spi_tx(uint8_t data)    // Transmit data 1 byte
{
    // Start tx
    SPDR = data;

    while(!(SPSR & (1<<SPIF)));  // Wait for tx complete //
    data = SPDR;

    return data;
}

uint8_t spi_rx(void)            // Read data 1 byte
{
    uint8_t data;
    SPDR = 0xff;          // dummy character but keep this 0xFF (high output)

    while(!(SPSR & (1<<SPIF))); // Wait for rx complete
    data = SPDR;

    return data;
}

//sdhc.c
uint8_t sdhc_init(void)
{
    uint8_t i, resp;
    uint16_t count = 0 ;

    SD_SS_INIT;

    SD_SS_DISABLE;

    for(i=0;i<10;i++)
        spi_tx(0xff);   //80 clock pulses minimum before first command //

    SD_SS_ENABLE;
    do
    {
        resp = sdhc_send_comm(GO_IDLE_STATE, 0);
        count++;
        if(count>0x20)
        return SDHC_ERR_TIMEOUT;
    } while(resp!=1);                    // resp of 1 => "idle state"

    SD_SS_DISABLE;
    spi_tx(0xff);                   // 8 clock cycle delay while SS line high
    spi_tx(0xff);                   // 8 clock cycle delay while SS line high

    count = 0;

    do
    {
        resp = sdhc_send_comm(SEND_IF_COND,0x000001AA); //resv only for SDHC card
        count++;
        if(count>0xfe)
            return SDHC_ERR_NOT_SDHC;
    } while(resp!=1);                    // resp of 1 => SDHC card

    count = 0;

    do
    {
        resp = sdhc_send_comm(APP_CMD, 0); //must be sent before any ACMD command
        resp = sdhc_send_comm(SD_SEND_OP_COND, 0x40000000); //ACMD41
        count++;
        if(count>0xfe)
            return SDHC_ERR_TIMEOUT_OP;
    } while(resp!=0);                     // resp of 0 => Success

    count = 0;

    do
    {
        resp = sdhc_send_comm(READ_OCR, 0);
        count++;
        if(count>0xfe)
            return SDHC_ERR_TIMEOUT_OCR;
    } while(resp != 0);                     // resp of 0 => Success

    return SDHC_ERR_NONE;
}

uint8_t sdhc_send_comm(uint8_t cmd, uint32_t arg)
{
    uint8_t resp, count = 0;

    SD_SS_ENABLE;
    spi_tx(cmd | 0x40); //send command, first two bits always '01'
    spi_tx(arg>>24);
    spi_tx(arg>>16);
    spi_tx(arg>>8);
    spi_tx(arg);

    if(cmd == SEND_IF_COND)
        spi_tx(0x87);           // CMD8 (CRC=0x87) - needed for CMD8
    else
        spi_tx(0x95);           // CMD0 (CRC=0x95) - all other CRC's ignored in SPI

    while((resp = spi_rx()) == 0xff)
    {  // wait response
        if(count++ > 0xfe)
            break;
    }

    if(resp == 0 && cmd == READ_OCR)
    {
        if((spi_rx() & 0x40) != 0x40)           // first byte of the OCR register (bit 31:24)
            resp = 1;               // return this for non-SDHC card

    spi_rx(); // (bits 23:16) low voltage power limits
    spi_rx(); // (bits 15:8) normal power supply limit
    spi_rx();   // (bits 7:0) reserved
    }

    spi_rx();       // extra 8 CLK
    SD_SS_DISABLE;

    return resp;
}

uint8_t sdhc_read_block(uint32_t startBlock)
{
    uint8_t resp;
    uint16_t i, count = 0;

    resp = sdhc_send_comm(READ_SINGLE_BLOCK, startBlock); //read a Block command

    if(resp != 0) return resp; // check for SD status: 0x00 - OK (No flags set)

    SD_SS_ENABLE;
    count = 0;

    while(spi_rx() != 0xfe)     // wait for start block token 11111110b
        if(count++ > 0xfffe)
        {
            SD_SS_DISABLE;
            return SDHC_ERR_R_TIMEOUT;
        }

    for(i = 0; i < 512; i++)    // read 512 bytes   .(GPC)  for(i = 9; i < 512; i++)
        buffer[i] = spi_rx();

    spi_rx();   // get CRC (16-bit), CRC not used
    spi_rx();

    spi_rx();   // 8 clock pulses

    SD_SS_DISABLE;

    return SDHC_ERR_NONE;
}

uint8_t sdhc_write_block(uint32_t startBlock)
{
    uint8_t resp;
    uint16_t i, count = 0;

    resp = sdhc_send_comm(WRITE_SINGLE_BLOCK, startBlock);

    if(resp != 0) return resp; // check resp for 0x00 - OK

    SD_SS_ENABLE;

    spi_tx(0xfe);               // start block token 11111110b

    for(i = 0; i < 512; i++)
        spi_tx(buffer[i]);

    spi_tx(0xff);               // bogus CRC (16-bit), CRC ignored
    spi_tx(0xff);

    resp = spi_rx();

    //resp = ***0xxx1b ; xxx='010' - ok;='101'- CRC error;='110'- write error
    if( (resp & 0x1f) != 0x05)
    {
        SD_SS_DISABLE;
        return resp;
    }

    while(!spi_rx())        // wait for write complete and go idle
        if(count++ > 0xfffe)
        {
            SD_SS_DISABLE;
            return SDHC_ERR_W_TIMEOUT;
        }

    SD_SS_DISABLE;

    spi_tx(0xff);               // 8 clock cycle delay while SS line high

    SD_SS_ENABLE;               // reset SS line low to verify if card still busy

    while(!spi_rx())        // wait for write complete and go idle
      if(count++ > 0xfffe)
      {
          SD_SS_DISABLE;
          return SDHC_ERR_W_TIMEOUT;
        }

    SD_SS_DISABLE;

    return SDHC_ERR_NONE;
}

That's it.

May 01, 2013
by pcbolt
pcbolt's Avatar

Garret -

That's really strange. The only two things I can think of is to test it again, maybe some wire was loose when one of the tests were made. The other is to check the header files again only this time check each header files that are included in other header files. Some of the header files list header files that list other that list others etc, etc...and can be buried 3 or 4 files deep. I don't think you'll find anything but who knows?

May 02, 2013
by Ralphxyz
Ralphxyz's Avatar

One can imagine that in the life of the GCC compiler "Done" might have been a key word with a reference buried deep in the compiler code somewhere.

I have actually seen this happen with certain variable names not working or causing errors.

Ralph

May 02, 2013
by scootergarrett
scootergarrett's Avatar

OK I think I have a handle on this problem so line 123 from code above (which shouldn't be commented out)

*((uint16_t*)&buffer[k*2]) = adc_read(0);

is stepping out of the memory for buffer and writing over other numbers (I thought I was good at pointers and this line had worked well for other things). So if I change line 82 from

     if(k*2>512)

to

if(k*2>=512)

I still don't understand why changing the variable name changed how it worked, the compiler must put them in different memory locations based on name. this is over my head. anyways this is for my Syncing up 2 or more microcontrollers project so I will post final result there soon. thanks every one again.

May 03, 2013
by BobaMosfet
BobaMosfet's Avatar

scootergarrett-

Think about how you're telling the compiler to view your array, and then ask yourself why you're using 'k*2'.

BM

May 05, 2013
by scootergarrett
scootergarrett's Avatar

The buffer needs to be char do the SDHC header will work sending 8 bits a time, but the 10 bit A/D need more space. I guess I could do something clever like

buffer[k] = ADCL;
buffer[k+1] = ADCH;
k += 2;
June 04, 2013
by BobaMosfet
BobaMosfet's Avatar

scootergarrett-

This line is your problem:

*((uint16_t*)&buffer[k*2]) = adc_read(0);

Because you are using 'k*2'. Why? In this line, you are telling the compiler that buffer[] is an array of ints, not bytes. So it automatically does the multiplication of the index by 2. But since you are multiplying... again.... you're way off the end of your buffer.

That's why I asked you to ask yourself why you were using 'k*2'.

BM

June 04, 2013
by BobaMosfet
BobaMosfet's Avatar

Also, 'done' is a keyword.

BM

June 04, 2013
by scootergarrett
scootergarrett's Avatar

The functions within #include "SDHC.h" needs the buffer to be 512 char to work so then I just run k from 0 to 256, so before I was only overflowing by 1 short. Now that its

if(k*2>=512)

it works fine and has been tested. That's interesting about the keyword.

Post a Reply

Please log in to post a reply.

Did you know that a thermometer can be made "faster" by using a bit of math? Learn more...