strings on avr

[carbon dude oxide]

hello, i am having trouble getting strings to work with atmel studio.

im currently re programming one of my arduino programs to be programmed in AVR GCC to be loaded onto an atmega328P what variable type do i used to replace String in arduino and how do i do functions such as index of?

[alm]

Neither AVR nor Atmel Studio are programming languages. You are presumably programming in C. Pick up a book/other resource about C and learn how strings are treated. Most of it is also applicable to small micros. If you are programming in C++ (which the Arduino IDE uses behind the scenes), then look up how strings work in C++.

[carbon dude oxide]

sorry yea i forgot to say i was using C

[senso]

Manual for the avr gcc compiler:
http://www.nongnu.org/avr-libc/user-manual/group__avr__string.html

[carbon dude oxide]

ive got it compiling with no errors now and i believe that i have codded it correctely just need to wait for my iscp programmer to arrive so i can program my avr with the code

thanks.

[bingo600]

Watch out ...

Defining "normal" strings will eat of your 2K ram (M328)

One could define the strings to reside in flash , where it won't eat up ram.
The keyword to put strings in flash is PROGMEM
const char *string PROGMEM =  "Test";

The you use the "normal" string routines , but with a _P at the end (_P for progmem)  - Note Capital _P
So a strcpy() would be strcpy_P()

Get a copy of the avr-libc reference manual here (the 1.8.0 manual expects your avr-gcc to use avr-libc v. 1.8.0)
http://download.savannah.gnu.org/releases/avr-libc/avr-libc-user-manual-1.8.0.tar.bz2
http://download.savannah.gnu.org/releases/avr-libc/avr-libc-user-manual-1.8.0.pdf.bz2

Code: [Select]


/**************************************************************************************
* Include section
***************************************************************************************/
#include <avr/io.h>
#include <avr/eeprom.h>
#include <avr/pgmspace.h>
#include <util/delay.h>
#include <inttypes.h>
#include <stdlib.h>
#include <string.h>


//
// Theese things are free from memory , but i think they all will work
//


char StringBufRam[80+1]; //Stringbuffer in Ram

const char *StringInFlash PROGMEM =  "Test"; //String in PROGMEM (Flash)


uint8_t str_temperature(char *str, uint16_t temperature); //Forward decl


main(void)
{
    strcpy_P(StringBufRam,StringInFlash);  //Now the flash string is copied to the Ram buffer , and can be handled with "non" _P routines
   //Or any other manipulation , including modifying the Ram-String contents

   
    printf_P("%s\n", StringInFlash);   //Tricky ... It saves the format string in PROGMEM (Flash) , so that one doesn't take up Ram Space
    printf_P("%s\n", PSTR("hello"));   //The PSTR macro automatically defines the string in Progmem (Flash) , neat for

    str_temperature(StringBufRam, 0x50);
}


//
// A Snip from my USB-AVR ds18b20 temp logger
//

const char ds18x20_fract[16][6] PROGMEM =  { //Array of strings in PROGMEM (Flash)
".0000", // 0/16 deg
".0625", // 1/16 deg
".1250", // 2/16 deg
".1875", // 3/16 deg
".2500", // 4/16 deg
".3125", // 5/16 deg
".3750", // 6/16 deg
".4375", // 7/16 deg
".5000", // 8/16 deg
".5625", // 9/16 deg
".6250", // 10/16 deg
".6875", // 11/16 deg
".7500", // 12/16 deg
".8125", // 13/16 deg
".8750", // 14/16 deg
".9375", // 15/16 deg
           };



//
// Builds a temperature string based on a ds18b20 reading
//
uint8_t str_temperature(char *str, uint16_t temperature)
{

// Called as str_temperature(s,sensor[SENSOR_INSIDE].raw_temp);

uint8_t degrees,fraction;
char s[10],*ptr,*fract_ptr;

ptr = str;

ptr = StringBufRam;

//Add degrees to string
degrees = (uint8_t) (temperature >> 4);
itoa(degrees,s,10);
strcat(ptr,s);

//Add Fraction to string (1/16 degrees)
fraction = (uint8_t) (temperature & 0x000f);
fract_ptr = (char *) ds18x20_fract[fraction];
strcat_P(ptr,fract_ptr);

return(strlen(ptr));
}


Basically maybe playy around with your strings as normal strings in Ram , get them to work.
Then try the PROGMEM versions , and see how it frees up Ram.

/Bingo

[David_AVD]

I went through this learning curve with C and Atmel Studio (for an Xmega) only recently.  The guys here helped me a lot.

Can you give an example of what you want to do and the errors you're getting?

[carbon dude oxide]

currently i am not getting any errors and i am storing my strings like this: char stringname[how_many_in_array][how_long_is_the_max_value+1]

i shall ahve alook into progmen in a bit

[David_AVD]

Do you have lots of strings?  If you have enough RAM I wouldn't worry too much about it.

In my project I am using an array of structures (one element is a string) and it works fine.  No PROGMEM required.

I have used PROGMEM for the font tables though as they are fairly large.  They never change so no need to hold them in RAM as well.

[IanB]

currently i am not getting any errors and i am storing my strings like this: char stringname[how_many_in_array][how_long_is_the_max_value+1]

i shall ahve alook into progmen in a bit

Note that the PROGMEM trick works for constant values, perhaps used for display or initialization, but it won't work for dynamically manipulating the string contents. For that the strings will need to be copied into RAM.

Also note that your

Code: [Select]

char stringname[how_many_in_array][how_long_is_the_max_value+1]
declaration will create a rectangular array, allocating space for the longest string even if it isn't used.

A more efficient use of memory would result from a "jagged" array, or an array of pointers, something like this:

Code: [Select]

char *stringarray[how_many_in_array]
This will give you an array of pointers, each of which (when initialized) can point to one of your strings. The strings can now be just as long as they need to be with no unused memory.

However, you will need now to allocate and manage the memory for each of the individual strings. More efficient use of memory leads to more work for the code to do.

[carbon dude oxide]

ok jut received my AVR ISP MKII in the post and went to program my vr, i made a copy of the hex of the program that was already on there and then proceeded to program, al went well except it did not do what i wanted it to (as always on the first go ) i kept all the fuses the same but it did not do anything unfortunatly.

the existing program on it was programmed using the arduino software (as the mc is currently in an arduino board)

here is the code currently:

just a handy pointer the reason the letters A-G are in there is so i knew which segment of the 7 segment display would light up

Code: [Select]

/*
 * PSUFrontDisplay.c
 *
 * Created: 02/09/2013 14:07:34
 *  Author: carbon dude oxide
 */


#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdio.h>
#include <string.h>

#define F_CPU 160000000UL
#define BUFFER_LENGTH 32
#define TWI_FREQ 1000000UL
#define TWI_BUFFER_LENGTH 32
#define TWI_READY 0
#define TWI_MRX   1
#define TWI_MTX   2
#define TWI_SRX   3
#define TWI_STX   4

 
 char displays[10][11] = {"X", "X", "G", "G", "G", "G", "G", "G", "G", "G"};
 char chars[12][8] = {"ABCDEF", "BC", "ABDEG", "ABCDG", "BCFG", "ADCFG", "ACDEFG", "ABC", "ABCDEFG", "ABCFG", "X", "G"};
 char portout[10][7] = {"X", "X", "PORTD2","PORTD3","PORTD4","PORTD5","PORTD6","PORTD7","PORTB0","PORTB1"};
 int portouthex[10] = {0x00, 0x00, 0x04, 0x08, 0x10, 0x20, 0x40, 0x80, 0x01, 0x02};

static volatile uint8_t twi_state;
static volatile uint8_t twi_slarw;
static volatile uint8_t twi_sendStop;
static volatile uint8_t twi_inRepStart;
   
uint8_t rxBuffer[BUFFER_LENGTH];
uint8_t rxBufferIndex = 0;
uint8_t rxBufferLength = 0;

uint8_t txAddress = 0;
uint8_t txBuffer[BUFFER_LENGTH];
uint8_t txBufferIndex = 0;
uint8_t txBufferLength = 0;

static void (*user_onRequest)(void);
static void (*user_onReceive)(int);
static void onRequestService(void);
static void onReceiveService(uint8_t*, int);
void onReceive( void (*)(int) );
void onRequest( void (*)(void) );
static void (*twi_onSlaveTransmit)(void);
static void (*twi_onSlaveReceive)(uint8_t*, int);

//static uint8_t twi_masterBuffer[TWI_BUFFER_LENGTH];
static volatile uint8_t twi_masterBufferIndex;
static volatile uint8_t twi_masterBufferLength;

//static uint8_t twi_txBuffer[TWI_BUFFER_LENGTH];
static volatile uint8_t twi_txBufferIndex;
static volatile uint8_t twi_txBufferLength;

//static uint8_t twi_rxBuffer[TWI_BUFFER_LENGTH];
static volatile uint8_t twi_rxBufferIndex;

static volatile uint8_t twi_error;


void twi_onReceive( void (*function)(int) )
{
user_onReceive = function;
}

void twi_onRequest( void (*function)(void) )
{
user_onRequest = function;
}

void onReceiveService(uint8_t* inBytes, int numBytes)
{
// don't bother if user hasn't registered a callback
if(!user_onReceive)
{
return;
}
// don't bother if rx buffer is in use by a master requestFrom() op
// i know this drops data, but it allows for slight stupidity
// meaning, they may not have read all the master requestFrom() data yet
if(rxBufferIndex < rxBufferLength)
{
return;
}
// copy twi rx buffer into local read buffer
// this enables new reads to happen in parallel
for(uint8_t i = 0; i < numBytes; ++i)
{
rxBuffer[i] = inBytes[i];
}
// set rx iterator vars
rxBufferIndex = 0;
rxBufferLength = numBytes;
// alert user program
user_onReceive(numBytes);
}
 
void onRequestService(void)
{
// don't bother if user hasn't registered a callback
if(!user_onRequest)
{
return;
}
// reset tx buffer iterator vars
// !!! this will kill any pending pre-master sendTo() activity
txBufferIndex = 0;
txBufferLength = 0;
// alert user program
user_onRequest();
}

void twi_attachSlaveRxEvent( void (*function)(uint8_t*, int) )
{
twi_onSlaveReceive = function;
}

void twi_attachSlaveTxEvent( void (*function)(void) )
{
twi_onSlaveTransmit = function;
}
 
void twi_Begin(uint8_t address)
{
TWAR = (address << 1);

twi_attachSlaveTxEvent(onRequestService);
twi_attachSlaveRxEvent(onReceiveService);

rxBufferIndex = 0;
rxBufferLength = 0;

txBufferIndex = 0;
txBufferLength = 0;

twi_state = TWI_READY;
twi_sendStop = 1; // default value
twi_inRepStart = 0;

PORTC = (1 << PORTC5 | PORTC4);

TWSR = (1 << TWPS0 | TWPS0);
TWBR = ((F_CPU / TWI_FREQ) - 16) / 2;

TWCR = (1 << TWEN | TWIE | TWEA);
}

int twi_available(void)
{
return rxBufferLength - rxBufferIndex;
}
 
int twi_read(void)
{
int value = -1;

// get each successive byte on each call
if(rxBufferIndex < rxBufferLength)
{
value = rxBuffer[rxBufferIndex];
++rxBufferIndex;
}

return value;
}
 
void receiveEvent(int howMany)
{
while(twi_available() > 4)
{
/*
-----structure of incoming bytes-----
Ba is currently unused (will be for negative numbers data)
Bb Bit 0+1 is position of DP on 7 segment display 1 and bit 2-7 is upper bits for display 1.
Bc lower bits for display 1 ( put together to make a 16bit display 1 number to show between 0 and 9999.
Bd Bit 0+1 is position of DP on 7 segment display 2 and bit 2-7 is upper bits for display 1.
Be lower bits for display 2 ( put together to make a 16bit display 2 number to show between 0 and 9999.
*/
 
char Ba = twi_read(); // receive byte as a character
char Bb = twi_read(); // receive byte as a character
char Bc = twi_read(); // receive byte as a character
char Bd = twi_read(); // receive byte as a character
char Be = twi_read(); // receive byte as a character
if (Ba == 0xFF)
{
//reset display info
strcpy(displays[2], "x");
strcpy(displays[3], "x");
strcpy(displays[4], "x");
strcpy(displays[5], "x");
strcpy(displays[6], "x");
strcpy(displays[7], "x");
strcpy(displays[8], "x");
strcpy(displays[9], "x");
 
// determine dp position by adding a mask and bit shifting to give a value between 0-3 to represent which digit to have the dp on
char dpp = Bb & 0xC0; // remove number data and leave DP position data
dpp = dpp >> 6; //bit shift to leave dp digit position
dpp = dpp + 2;
int idpp = dpp;
strcpy(displays[idpp], strcat(displays[idpp],"P"));
//  displays[testb] = displays[testb]+"P"; //add P to display dp in corresponding digit
Bb = Bb & 0x3F; //remove DP Position data and leave number data
 
short lb = (Bb << 8) | Bc; // combine upper and lower byte to form 16bit number data
int fbT;
int fbh;
int fbt;
int fbu;
if (lb <= 9999) //if received data wants to display number larger than 9999 then display ----
{
//decode binary into 4 individual digits.
short lbT = lb/1000;
fbT = lbT;
if ((fbT >= 10) || (fbT <= 0)) {fbT = 10;}
short lbh = (lb-(lbT*1000))/100;
fbh = lbh;
short lbt = (lb-(lbh*100)-(lbT*1000))/10;
fbt = lbt;
short lbu = (lb-(lbh*100)-(lbt*10)-(lbT*1000));
fbu = lbu;
}
else
{
//set digits as ----
fbT = 11;
fbh = 11;
fbt = 11;
fbu = 11;
}
// determine dp position by adding a mask and bit shifting to give a value between 0-3 to represent which digit to have the dp on
short dppb = Bd & 0xC0;
dppb = dppb >> 6;// remove number data and leave DP position data
dppb = dppb + 6;
int idppb = dppb;
strcpy(displays[idppb], strcat(displays[idppb],"P"));
//  displays[dppb] = displays[dppb]+"P";//add P to display dp in corresponding digit
Bd = Bd & 0x3F;//remove DP Position data and leave number data
 
short lbb = (Bd << 8) | Be;
int fbTb;
int fbhb;
int fbtb;
int fbub;

if (lbb <= 9999)//if received data wants to display number larger than 9999 then display ----
{
//decode binary into 4 individual digits.
short lbTb = lbb/1000;
fbTb = lbTb;
if ((fbTb >= 10) || (fbTb <= 0)) {fbTb = 10;}
short lbhb = (lbb-(lbTb*1000))/100;
fbhb = lbhb;
short lbtb = (lbb-(lbhb*100)-(lbTb*1000))/10;
fbtb = lbtb;
short lbub = (lbb-(lbhb*100)-(lbtb*10)-(lbTb*1000));
fbub = lbub;
}
else
{
//set digits as ----
fbTb = 11;
fbhb = 11;
fbtb = 11;
fbub = 11;
}
 
//add character data for each digit in both displays
 
strcpy(displays[2], strcat(displays[2],chars[fbu]));
strcpy(displays[3], strcat(displays[3],chars[fbt]));
strcpy(displays[4], strcat(displays[4],chars[fbh]));
strcpy(displays[5], strcat(displays[5],chars[fbT]));
strcpy(displays[6], strcat(displays[6],chars[fbub]));
strcpy(displays[7], strcat(displays[7],chars[fbtb]));
strcpy(displays[8], strcat(displays[8],chars[fbhb]));
strcpy(displays[9], strcat(displays[9],chars[fbTb]));
}
}
}
 
void setup()
{
DDRB = (1 << PORTB0 | PORTB1 | PORTB2 | PORTB3 | PORTB4 | PORTB5);
DDRC = (1 << PORTC0 | PORTC1 | PORTC2 | PORTC3);
DDRD = (1 << PORTD2 | PORTD3 | PORTD4 | PORTD5 | PORTD6 | PORTD7);
twi_Begin(2);                // join i2c bus with address #2
twi_onReceive(receiveEvent); // register event to handle requests
}

int main()
{
// put your main code here, to run repeatedly:
//Serial.println(',', DEC);
for (int i=9; i>1; i--)
{
//set all display ports low and leave other outputs
PORTB = PORTB & 0x03;
PORTC = PORTC & 0x0F;
PORTD = PORTD & 0xC0;
 
if (strstr(displays[i],"A") != NULL) {PORTC = (1 <<  PORTC1);}
if (strstr(displays[i],"B") != NULL) {PORTC = (1 <<  PORTC0);}
if (strstr(displays[i],"C") != NULL) {PORTC = (1 <<  PORTC3);}
if (strstr(displays[i],"D") != NULL) {PORTB = (1 <<  PORTB4);}
if (strstr(displays[i],"E") != NULL) {PORTB = (1 <<  PORTB5);}
if (strstr(displays[i],"F") != NULL) {PORTB = (1 <<  PORTB2);}
if (strstr(displays[i],"G") != NULL) {PORTB = (1 <<  PORTB3);}
if (strstr(displays[i],"P") != NULL) {PORTC = (1 <<  PORTC2);}

if (strstr(portout[i],"B") != NULL) {PORTB = PORTB | portouthex[i];}
if (strstr(portout[i],"C") != NULL) {PORTC = PORTC | portouthex[i];}
if (strstr(portout[i],"D") != NULL) {PORTD = PORTD | portouthex[i];}
}
}
the code has snippets from the arduino wire library so that i can use I2C on it.

the program is meant to wait for 5 incoming I2C data bytes and then decode the 5 bytes into numbers for 2 lots of 4 7-segment displays there are commends near the byte decoding to show what each bit is for and im prety sure this could be done more efficiently but first i would like to get it working

here is the original arduino program (for reference) which worked fine:

Code: [Select]

#define A A1
#define B A0
#define C A3
#define D 12
#define E 13
#define f 10
#define G 11
#define P A2
#define I1 5
#define I2 4
#define I3 3
#define I4 2
#define V1 9
#define V2 8
#define V3 7
#define V4 6

String displays[] = {"X", "X", "G", "G", "G", "G", "G", "G", "G", "G"};
String chars[] = {"ABCDEF", "BC", "ABDEG", "ABCDG", "BCFG", "ADCFG", "ACDEFG", "ABC", "ABCDEFG", "ABCFG", "X", "G"};

int noob = 0;
byte startbyte;
byte endbyte;

void setup() {
  // put your setup code here, to run once:
  pinMode(I1, OUTPUT);
  pinMode(I2, OUTPUT);
  pinMode(I3, OUTPUT);
  pinMode(I4, OUTPUT);
  pinMode(V1, OUTPUT);
  pinMode(V2, OUTPUT);
  pinMode(V3, OUTPUT);
  pinMode(V4, OUTPUT);
 
  pinMode(A, OUTPUT);
  pinMode(B, OUTPUT);
  pinMode(C, OUTPUT);
  pinMode(D, OUTPUT);
  pinMode(E, OUTPUT);
  pinMode(f, OUTPUT);
  pinMode(G, OUTPUT);
  pinMode(P, OUTPUT);
  Wire.begin(2);                // join i2c bus with address #2
  Wire.onReceive(receiveEvent); // register event to handle requests
 
}

void loop() {
  // put your main code here, to run repeatedly:
  //Serial.println(',', DEC);
  for (int i=9; i>1; i--) {
    digitalWrite(2, LOW);
    digitalWrite(3, LOW);
    digitalWrite(4, LOW);
    digitalWrite(5, LOW);
   
    digitalWrite(6, LOW);
    digitalWrite(7, LOW);
    digitalWrite(8, LOW);
    digitalWrite(9, LOW);
   
    digitalWrite(A, LOW);
    digitalWrite(B, LOW);
    digitalWrite(C, LOW);
    digitalWrite(D, LOW);
    digitalWrite(E, LOW);
    digitalWrite(f, LOW);
    digitalWrite(G, LOW);
    digitalWrite(P, LOW);
   
    if (displays[i].indexOf('A') > -1) {digitalWrite(A, HIGH);}
    if (displays[i].indexOf('B') > -1) {digitalWrite(B, HIGH);}
    if (displays[i].indexOf('C') > -1) {digitalWrite(C, HIGH);}
    if (displays[i].indexOf('D') > -1) {digitalWrite(D, HIGH);}
    if (displays[i].indexOf('E') > -1) {digitalWrite(E, HIGH);}
    if (displays[i].indexOf('F') > -1) {digitalWrite(f, HIGH);}
    if (displays[i].indexOf('G') > -1) {digitalWrite(G, HIGH);}
    if (displays[i].indexOf('P') > -1) {digitalWrite(P, HIGH);}
    digitalWrite(i, HIGH);
    delay(1);
  }
}

void receiveEvent(int howMany)
{
  while(Wire.available() > 4)
  {
    byte Ba = Wire.read(); // receive byte as a character
    byte Bb = Wire.read(); // receive byte as a character
    byte Bc = Wire.read(); // receive byte as a character
    byte Bd = Wire.read(); // receive byte as a character
    byte Be = Wire.read(); // receive byte as a character
    if (Ba == B11111111) {
      displays[2] = "x";
      displays[3] = "x";
      displays[4] = "x";
      displays[5] = "x";
      displays[6] = "x";
      displays[7] = "x";
      displays[8] = "x";
      displays[9] = "x";
     
      byte dpp = Bb & B11000000;
      dpp = dpp >> 6;
      dpp = dpp + 2;
      displays[dpp] = displays[dpp]+"P";
      Bb = Bb & B00111111;
     
      word lb = (Bb << 8) | Bc;
      int fbT;
      int fbh;
      int fbt;
      int fbu;
      if (lb <= 9999)
      {
        word lbT = lb/1000;
        fbT = lbT;
        if ((fbT >= 10) || (fbT <= 0)) {fbT = 10;}
        word lbh = (lb-(lbT*1000))/100;
        fbh = lbh;
        word lbt = (lb-(lbh*100)-(lbT*1000))/10;
        fbt = lbt;
        word lbu = (lb-(lbh*100)-(lbt*10)-(lbT*1000));
        fbu = lbu;
      }
      else
      {
        fbT = 11;
        fbh = 11;
        fbt = 11;
        fbu = 11;
      }
     
      byte dppb = Bd & B11000000;
      dppb = dppb >> 6;
      dppb = dppb + 6;
      displays[dppb] = displays[dppb]+"P";
      Bd = Bd & B00111111;
       
      word lbb = (Bd << 8) | Be;
      int fbTb;
      int fbhb;
      int fbtb;
      int fbub; 
     
      if (lbb <= 9999)
      {
        word lbTb = lbb/1000;
        fbTb = lbTb;
        if ((fbTb >= 10) || (fbTb <= 0)) {fbTb = 10;}
        word lbhb = (lbb-(lbTb*1000))/100;
        fbhb = lbhb;
        word lbtb = (lbb-(lbhb*100)-(lbTb*1000))/10;
        fbtb = lbtb;
        word lbub = (lbb-(lbhb*100)-(lbtb*10)-(lbTb*1000));
        fbub = lbub;
      }
      else
      {
        fbTb = 11;
        fbhb = 11;
        fbtb = 11;
        fbub = 11;
      }
 
      displays[2] = displays[2] + chars[fbu];
      displays[3] = displays[3] + chars[fbt];
      displays[4] = displays[4] + chars[fbh];
      displays[5] = displays[5] + chars[fbT];
      displays[6] = displays[6] + chars[fbub];
      displays[7] = displays[7] + chars[fbtb];
      displays[8] = displays[8] + chars[fbhb];
      displays[9] = displays[9] + chars[fbTb];
    }
  }
}

Unfortunately i am unsure why it is not working, avr studio is not coming up with any compiling errors and is programming the avr fine (unless im not setting up the fuses correctly or there i the wrong boot loader on it?)

any help on this would be brilliant thanks

[David_AVD]

I don't know anything about I2C on the AVR so no help there.

Have you written any other software in ATMEL Studio to test on the chip?  If not, a blinking LED test program would be a good start so you know you have the AS part right.

[carbon dude oxide]

yes i have written other programs and they have successfully been programmed into the AVR, i have been tweaking my code for the  past few hours and i have narrowed it down to the I2C code. its a bit late for me to continue with it tonight but if anyone knows of any pre made AVR Library for I2C on the ATMega328P to enable me to receive data then please let me know otherwise i will be continuing to tweak the code (i am currently attempting to port over the Wire library from Arduino to Atmel studio so i shall see how that goes in the morning :/

[senso]

I2C Master Interface, scroll down a bit to see the download link:
http://homepage.hispeed.ch/peterfleury/avr-software.html

Read the info, use the lib, works perfectly out of the box.

[dannyf]

I would suggest that you isolate your code into two pieces: one to receive data and one to display the data. Your goal should be to get each one to work independently and then go from there.

now, if you look at your display piece, it has numerous assumptions and you will need to know if they are valid for your particular set-up.

Some suggestions:
1) the code is poorly constructed: many ports / pins are hard coded, making adapting them difficult in the future;
2) the structure makes no sense to me: you should read on state machines and see how you could rewrite your code that way;
3) you should comment on your code extensively so in the future you know what you are doing;
4) think about how the code can be written so that you can benefit from it in other projects.

I am happy to help you write a piece but you have to do some heavy lifting yourself.

[dannyf]

The general concept is this:

Code: [Select]

  set up the segment / digit pins to output;
  while (1) {
    turn off all digit pins; //avoid blinking digits
    turn on the segment pins based on display buffer + digit index;
    turn on the digit pointed to by the digit index;
    increment to the next digit - need to check for boundary
  }

The display buffer can either store the segment information, or display content.

Once you have it done, you can figure out a way to run it in a timer isr so the whole display thing can be transparent to you.

[carbon dude oxide]

I2C Master Interface, scroll down a bit to see the download link:
http://homepage.hispeed.ch/peterfleury/avr-software.html

Read the info, use the lib, works perfectly out of the box.

Yes i did have a look at using that one before but there is no facility to use te avr as a slave (which is what i am trying to do unfortunatly

[carbon dude oxide]

I would suggest that you isolate your code into two pieces: one to receive data and one to display the data. Your goal should be to get each one to work independently and then go from there.

now, if you look at your display piece, it has numerous assumptions and you will need to know if they are valid for your particular set-up.

Some suggestions:
1) the code is poorly constructed: many ports / pins are hard coded, making adapting them difficult in the future;
2) the structure makes no sense to me: you should read on state machines and see how you could rewrite your code that way;
3) you should comment on your code extensively so in the future you know what you are doing;
4) think about how the code can be written so that you can benefit from it in other projects.

I am happy to help you write a piece but you have to do some heavy lifting yourself.

Unfortunatly hard coding my ports and pins is the only way i know how to use them (apart from using _BV() but it messed up the code unfortunately (more than it is already . Ok i am unsure about state machines and how i would use them... I have had a look at a couple of websites but i am still unsure unfortunatly, The code that i have at the moment is a bit more organised than it was before, i will also go through and comment all the lines and repost the new code after i have cleaned it up a bit more.

[carbon dude oxide]

ok I have cleaned the code up a bit and commented almost everything. the TwoWire. functions will not be in there in an hour or so as they are from the arduino code that i made.

im going to e making a new I2C library as i am unable to find a slave one on-line :/ it will be based off of the arduino Wire library

Code: [Select]

/*
 * PSUFrontDisplay.c
 *
 *  Created: 02/09/2013 14:07:34
 *   Author: carbon dude oxide
 * Avr used: ATMega328p
 */

/* Circuit Structure

7-Segment layout:
Common Anode displays

      A
    ------
F  |      |  B
   |  G   |
    ------
E  |      |  C
   |  D   |
    ------ .P (DP)

Circuit is laid out like so:
the line above each 7-segment display shows which pin on the AVR the common anode is connected to,
the number in brackets corresponds to the ID number of each display used in arrays (index number) and for loops.
There are two sets of 4 displays with all their A segments connected together and all the B segments
together to perform a multiplexing task.

---------------------------------
 PD1 (0) PD0 (1) PD7 (2) PD6 (3)
|  ---  |  ---  |  ---  |  ---  |
| |   | | |   | | |   | | |   | |
|  ---  |  ---  |  ---  |  ---  |
| |   | | |   | | |   | | |   | |
|  ---  |  ---  |  ---  |  ---  |
---------------------------------
 PD5 (4) PD4 (5) PD3 (6) PD2 (7)
|  ---  |  ---  |  ---  |  ---  |
| |   | | |   | | |   | | |   | |
|  ---  |  ---  |  ---  |  ---  |
| |   | | |   | | |   | | |   | |
|  ---  |  ---  |  ---  |  ---  |
---------------------------------

each AVR pin used to control the segments is wired to an NPN transistor which in turn pulls each cathode
of the 7-Segment display low allowing the LED to light

A segments connected to pin: PC1
B segments connected to pin: PC0
C segments connected to pin: PC3
D segments connected to pin: PB4
E segments connected to pin: PB5
F segments connected to pin: PB2
G segments connected to pin: PB3
P segments connected to pin: PC2 (P is decimal point)

*/

#define F_CPU 16000000UL  // 16 MHz clock frequency

#include <avr/io.h> //standard library loaded
#include <util/delay.h> //needed for delay on multiplexing
#include <stdio.h> //used for current method of displaying data
#include <string.h> //used for current method of displaying data
#include <custom/Wire/Wire.h> //modified Wire library from Arduino

/* displays data is used to determine which segment to light up on each 7 segment display
 *
 */
char displays[8][11] = {"G", "G", "G", "G", "G", "G", "G", "G"};

/* chars data is which segments should be lit up when wanting to display a number for example:
 * the number 0 needs "ABCDEF" segments to be lit
 * the number 1 needs "BC" segments to be lit
 * in the array there are two extra entries equating to numbers 10 and 11
 * 10 is used for a blank display and 11 is used to display a negative symbol
 */
char chars[12][8] = {"ABCDEF", "BC", "ABDEG", "ABCDG", "BCFG", "ADCFG", "ACDEFG", "ABC", "ABCDEFG", "ABCFG", "X", "G"};


int main(void)
{
/* Set all used pins to outputs
* PD1, PD0, PD7, PD6, PD5, PD4, PD3 and PD2 for anodes for each of the 8 7-segment displays and
* PC1, PC0, PC3, PB4, PB5, PB2, PB3 and PC2 for NPN transistor for each segment of the 7-segment displays
*/
DDRB = (1 << PORTB0) | (1 << PORTB1) | (1 << PORTB2) | (1 << PORTB3) | (1 << PORTB4) | (1 << PORTB5);
DDRC = (1 << PORTC0) | (1 << PORTC1) | (1 << PORTC2) | (1 << PORTC3);
DDRD = (1 << PORTD0) | (1 << PORTD1) | (1 << PORTD2) | (1 << PORTD3) | (1 << PORTD4) | (1 << PORTD5) | (1 << PORTD6) | (1 << PORTD7);
//initiate the I2C Protocol
Wire.begin(2);                // join i2c bus with address #2
Wire.onReceive(receiveEvent); // register event to handle requests

//the loop function for multiplexing
while(1)
{
for (int dID=0; dID<8; dID++) //cycle through display ID's 0-7 :D
{
//set all used display ports low and ignore other outputs
PORTB = PORTB & 0xC0; // set all ports except PORTB6 and PORTB7 low
PORTC = PORTC & 0x0F; // set all ports except PORTC4, PORTB5, PORTC6 and PORTB7 low
PORTD = 0x00; // set all ports low
 
/* scan the displays string data for a used letter example and turn the corresponding output on:
* displays[0] (for display id:0) contains "BC" which corresponds to the number 1 to be displayed (from chars array)
* this code will scan the string for each letter and come true for B and C so will turn on the B and C segments on all the displays
* but due to all the anodes being off (from last few lines of script) the segments will not light
*/
if (strstr(displays[dID],"A") != NULL) {PORTC = (1 << PORTC1);}
if (strstr(displays[dID],"B") != NULL) {PORTC = (1 << PORTC0);}
if (strstr(displays[dID],"C") != NULL) {PORTC = (1 << PORTC3);}
if (strstr(displays[dID],"D") != NULL) {PORTB = (1 << PORTB4);}
if (strstr(displays[dID],"E") != NULL) {PORTB = (1 << PORTB5);}
if (strstr(displays[dID],"F") != NULL) {PORTB = (1 << PORTB2);}
if (strstr(displays[dID],"G") != NULL) {PORTB = (1 << PORTB3);}
if (strstr(displays[dID],"P") != NULL) {PORTC = (1 << PORTC2);}
       
/* Now that all the segment data has been sifted through and has turned on each segment to be lit up for the display
* this is done by asking which dID (display ID) is currently being used in the for loop and then turning on the output
* for that 7-Segment display, in the previous chunk of code the segments were selected and in this chunk the 7-segment display
* is selected and tada it lights up
*/
if (dID == 0) {PORTD = (1 << PORTD1);}
else if (dID == 1) {PORTD = (1 << PORTD0);}
else if (dID == 2) {PORTD = (1 << PORTD7);}
else if (dID == 3) {PORTD = (1 << PORTD6);}
else if (dID == 4) {PORTD = (1 << PORTD5);}
else if (dID == 5) {PORTD = (1 << PORTD4);}
else if (dID == 6) {PORTD = (1 << PORTD3);}
else if (dID == 7) {PORTD = (1 << PORTD2);}
_delay_ms(1); //delay 1ms so that it does not go too crazy and lights up the segments correctly
}
}
}

void receiveEvent(int howMany) //custom function to be called when I2C slave address is acknowledged and is receiving bytes of data
{
while(Wire.available() > 4) //check to see if there are 5 bytes available in storage
{
/*
-----structure of incoming bytes-----
Ba is currently unused (will be for negative numbers data)
Bb Bit 0+1 is position of DP on 7 segment display 1 and bit 2-7 is upper bits for display 1.
Bc lower bits for display 1 ( put together to make a 16bit display 1 number to show between 0 and 9999.
Bd Bit 0+1 is position of DP on 7 segment display 2 and bit 2-7 is upper bits for display 2.
Be lower bits for display 2 ( put together to make a 16bit display 2 number to show between 0 and 9999.
*/
 
char Ba = TwoWire.read(); // receive byte as a character
char Bb = TwoWire.read(); // receive byte as a character
char Bc = TwoWire.read(); // receive byte as a character
char Bd = TwoWire.read(); // receive byte as a character
char Be = TwoWire.read(); // receive byte as a character
if (Ba == 0xFF)
{
//reset display info by setting displays arrays to X (blank display)
strcpy(displays[2], "x");
strcpy(displays[3], "x");
strcpy(displays[4], "x");
strcpy(displays[5], "x");
strcpy(displays[6], "x");
strcpy(displays[7], "x");
strcpy(displays[8], "x");
strcpy(displays[9], "x");
 
// determine dp position by adding a mask and bit shifting to give a value between 0-3 to represent which digit to have the dp on
// look at structure of incoming bytes to see why i bit shift and stuff
char dpp = Bb & 0xC0; // remove number data and leave DP position data
dpp = dpp >> 6; //bit shift to leave dp digit position a binary value between 0-3 (corresponding to which digit to display the DP on)
int idpp = 3 - dpp; // invert the displayID (as it is the wrong way round) and convert into a int corresponding to the 4 displays on the 1st block of displays (ID's 0-3)
strcpy(displays[idpp], strcat(displays[idpp],"P")); //add the letter P to the specific dID array which is calculated in the last few lines
Bb = Bb & 0x3F; //remove DP Position data and leave number data to we can calculate what number to be displays on the digits
 
short lb = (Bb << 8) | Bc; // combine upper and lower byte to form 16bit number data (so that we can have a number between 0 and 9999)
// set up individual numbers for each of the 4 displays in the 1st of the 2 blocks of 4 displays
int fbT;
int fbh;
int fbt;
int fbu;
if (lb <= 9999) //if received data wants to display number larger than 9999 then display ----
{
//decode binary into 4 individual digits.
short lbT = lb/1000;
fbT = lbT;
if ((fbT >= 10) || (fbT <= 0)) {fbT = 10;}
short lbh = (lb-(lbT*1000))/100;
fbh = lbh;
short lbt = (lb-(lbh*100)-(lbT*1000))/10;
fbt = lbt;
short lbu = (lb-(lbh*100)-(lbt*10)-(lbT*1000));
fbu = lbu;
}
else
{
//set digits as ---- because 11 corresponds to G segment being lit (view chars[11] array)
fbT = 11;
fbh = 11;
fbt = 11;
fbu = 11;
}


// determine dp position (for second block of displays) by adding a mask and bit shifting to give a value between 0-3 to represent which digit to have the dp on
// look at structure of incoming bytes to see why i bit shift and stuff
short dppb = Bd & 0xC0; // remove number data and leave DP position data
dppb = dppb >> 6; //bit shift to leave dp digit position a binary value between 0-3 (corresponding to which digit to display the DP on)
int idppb = 7 - dppb; // invert the displayID (as it is the wrong way round) and convert into a int corresponding to the 4 displays on the 2nd block of displays (ID's 4-7)
strcpy(displays[idppb], strcat(displays[idppb],"P")); //add the letter P to the specific dID array which is calculated in the last few lines
Bd = Bd & 0x3F;  //remove DP Position data and leave number data to we can calculate what number to be displays on the digits
 
short lbb = (Bd << 8) | Be; // combine upper and lower byte to form 16bit number data (so that we can have a number between 0 and 9999)
// set up individual numbers for each of the 4 displays in the 2nd of the 2 blocks of 4 displays
int fbTb;
int fbhb;
int fbtb;
int fbub;

if (lbb <= 9999)//if received data wants to display number larger than 9999 then display ----
{
//decode binary into 4 individual digits.
short lbTb = lbb/1000;
fbTb = lbTb;
if ((fbTb >= 10) || (fbTb <= 0)) {fbTb = 10;}
short lbhb = (lbb-(lbTb*1000))/100;
fbhb = lbhb;
short lbtb = (lbb-(lbhb*100)-(lbTb*1000))/10;
fbtb = lbtb;
short lbub = (lbb-(lbhb*100)-(lbtb*10)-(lbTb*1000));
fbub = lbub;
}
else
{
//set digits as ---- because 11 corresponds to G segment being lit (view chars[11] array)
fbTb = 11;
fbhb = 11;
fbtb = 11;
fbub = 11;
}
 
/* add character data for each digit in both displays
* This works by getting the segment data from the chars array acording to the number calculated for each digit from the incomming I2C data
* It is then added onto the existing data that is already in dispays (the strcat() function) and then finaly is then inserted into the displays
* array where it replaces the existing data (hence the need to copy the existing data and add it to the new data before replacing it
*/
 
strcpy(displays[0], strcat(displays[0],chars[fbu]));
strcpy(displays[1], strcat(displays[1],chars[fbt]));
strcpy(displays[2], strcat(displays[2],chars[fbh]));
strcpy(displays[3], strcat(displays[3],chars[fbT]));
strcpy(displays[4], strcat(displays[4],chars[fbub]));
strcpy(displays[5], strcat(displays[5],chars[fbtb]));
strcpy(displays[6], strcat(displays[6],chars[fbhb]));
strcpy(displays[7], strcat(displays[7],chars[fbTb]));
}
}
}


[Kremmen]

Some lightning comments, more may be on the way:

1:
char displays[8][11] = {"G", "G", "G", "G", "G", "G", "G", "G"};
This may be suboptimal. If you only need an array of 8 chars, why have a 2 dimensional matrix? Waste of RAM for nothing.

2:
for (int dID=0; dID<8; dID++) //cycle through display ID's 0-7
      {
         //set all used display ports low and ignore other outputs
         PORTB = PORTB & 0xC0; // set all ports except PORTB6 and PORTB7 low
         PORTC = PORTC & 0x0F; // set all ports except PORTC4, PORTB5, PORTC6 and PORTB7 low
         PORTD = 0x00; // set all ports low
etc etc
Remove the static port setups from the loop. You don't want to keep repeating that for nothing.

3:
if (strstr(displays[dID],"A") != NULL) {PORTC = (1 << PORTC1);}

Man. This must be the most convoluted way of checking for a char i have seen in a long time.
How about if you change the whole thing to look something like this:

char displays[8] = {'G', 'G, ...};
...
...
if ( displays[dID] == 'A' ) {...}

[carbon dude oxide]

Some lightning comments, more may be on the way:

1:
char displays[8][11] = {"G", "G", "G", "G", "G", "G", "G", "G"};
This may be suboptimal. If you only need an array of 8 chars, why have a 2 dimensional matrix? Waste of RAM for nothing.

its not just an array of 8 chars its an array with 8 entries with each containing a maximum of 10 chars in each. the letters get inserted when it receives some data from the I2C bus. its only got G's in it initaly so the displays only have a - on them. i will soon be converting to pgmemory to free up the ram (probably not going to do it until i have fixed the I2C issue)

2:
for (int dID=0; dID<8; dID++) //cycle through display ID's 0-7
      {
         //set all used display ports low and ignore other outputs
         PORTB = PORTB & 0xC0; // set all ports except PORTB6 and PORTB7 low
         PORTC = PORTC & 0x0F; // set all ports except PORTC4, PORTB5, PORTC6 and PORTB7 low
         PORTD = 0x00; // set all ports low
etc etc
Remove the static port setups from the loop. You don't want to keep repeating that for nothing.

i am unsure what you mean by Remove the static port setups unfortunately... could you give me an example?

3:
if (strstr(displays[dID],"A") != NULL) {PORTC = (1 << PORTC1);}

Man. This must be the most convoluted way of checking for a char i have seen in a long time.
How about if you change the whole thing to look something like this:

char displays[8] = {'G', 'G, ...};
...
...
if ( displays[dID] == 'A' ) {...}

i did that because its going to be an array with multiple characters within each entry (up to 10) and each one could contain all the letters so i did it this way? again like i said before i will be converting to pgmemory soon but fornow i had just ported my arduino program to AVR and this is what i found to replace strings and stuff liek that. it works for now but it can definatly be made more efficiant

[dannyf]

ok I have cleaned the code up a bit

Good progress.

Here is what I would do in the main loop:

Code: [Select]

#define DIG_PORT  PORTD  //digits on portd
#define DIG0          (1<<1)  //dig0 on pin0
#define DIG1          (1<<0)  //dig1 on pin1
...
#define DIG7          (1<<2)  //dig7 on pin2
#define DIG_ON(digs)  DIG_PORT |= (digs) //turn on digs
#define DIG_OFF(digs) DIG_PORT &=~(digs) //turn off digs

  //in the while loop
  //turn off all digits
  switch (dID) {
  case 0: DIG_OFF(DIG7); SEG(display[dID]); DIG_ON(DIG0); dID=1; break; //light up digit 0
  case 1: DIG_OFF(DIG0); SEG(display[dID]); DIG_ON(DIG1); dID=2; break; //light up digit 1
  ...
  case 7: DIG_OFF(DIG6); SEG(display[dID]); DIG_ON(DIG7); dID=0; break; //light up digit 7
  }

All is left is to write SEG() so it lights the right segments.

[carbon dude oxide]

ok I have cleaned the code up a bit

Good progress.

Here is what I would do in the main loop:

Code: [Select]

#define DIG_PORT  PORTD  //digits on portd
#define DIG0          (1<<1)  //dig0 on pin0
#define DIG1          (1<<0)  //dig1 on pin1
...
#define DIG7          (1<<2)  //dig7 on pin2
#define DIG_ON(digs)  DIG_PORT |= (digs) //turn on digs
#define DIG_OFF(digs) DIG_PORT &=~(digs) //turn off digs

  //in the while loop
  //turn off all digits
  switch (dID) {
  case 0: DIG_OFF(DIG7); SEG(display[dID]); DIG_ON(DIG0); dID=1; break; //light up digit 0
  case 1: DIG_OFF(DIG0); SEG(display[dID]); DIG_ON(DIG1); dID=2; break; //light up digit 1
  ...
  case 7: DIG_OFF(DIG6); SEG(display[dID]); DIG_ON(DIG7); dID=0; break; //light up digit 7
  }

All is left is to write SEG() so it lights the right segments.

could you explain how this works please?

[dannyf]

DIG_ON() turns a digit on by setting that pin on DIG_PORT(PORTD here), following your convention; DIG_OFF() clears that pin then turning the digit off.

For each digit, it turns off the previously lite digit, turn on the segment information for the current digit, and then turn on the current digit and then rotates through all the digits.

As simple as that.

If I were you, I would put this in a function and make dID static so each time you call this function, the display is updated.

[carbon dude oxide]

wait,done a bit of reading and i think i understand it

the first #defines before the DIG_ON are the output pins the DIG_ON uses a Inclusive OR to turn on digit (i am unsure how to knows to do this in the #define though that you will have to explain and the DIG_OFF just turns off that Digit using Not And

il have a look into writing the SEG() command thingy.

and another question this is state machines correct?

edit: oh i didnt realise you had posted