Help with basic ATmega328p serial comms

Go To Last Post
44 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

The code below started out as an Arduino sketch I wrote as an exercise, teaching myself about and bits and bytes but turned into a "Knight Rider" effect with LED's.

I have become fascinated with how the 328p works and decided to try and replace the arduino stuff like "digital Write" and "analog Read" by using direct manipulation of the registers, and so with time, google and the data sheet I have begun to understand more and more and I am thoroughly enjoying it! I then decided to use the ADC and a pot to vary the speed,  I thought it would be great to output the result of the ADC to the serial monitor. It  is here that I have hit a sort of block, I think I am going to need to change the data type maybe with a command like "char" or something like that.

In the code I have put what I thought would work, simply putting the result from ADC0H into UDR0 with "UDR0 = ADCH;" but this is wrong, the line below it puts a character into the UDR0 register and that's ok so my setup is working of sorts. I am very new to coding and am plodding along at my own pace, but I can't seem to get my head around this for some reason, this whole thing is just for fun, and upto now have managed to think it through, but would appreciate some guidance here, as I don't get it!...cheers.  

#define F_CPU 1000000L
#include <avr/io.h>
#include <util/delay.h>

int Led_number;
int Counter;
int Delay = 15;
bool Direction;

void delay_ms( int ms );

int main(void)
{
    DDRB = 0xff;

    //***************** ADC0 is used here and is set by default *******************
    ADMUX |= 1 << REFS0; // This sets our reference to the 5v input rail
	ADMUX |= 1 << ADLAR; // This moves the result into the high byte of ADCH for 8 bit
	ADCSRA |= 1 << ADEN; // This Enables the ADC

	//********* This is setting up USART to send the potentiometer value serialy ***
	UBRR0 = 12; // this sets baud rate at 9600
	UCSR0A = (1 << U2X0); // this doubles data rate in asynchronous mode
	UCSR0B = (1 << TXEN0); // This enables transmit
	UCSR0C = (1 << UCSZ01) | (1 << UCSZ00); // This sets character size to 8 bits

    while (1)
    {
		  for ( Counter = 1; Counter <= 7; Counter ++)
		  {
			  if (Direction == 0)
			  {
				  PORTB = (1 << Led_number); // This shifts a 1 along the register
				  ADCSRA |= 1 << ADSC; // This Starts Conversion
				  delay_ms (Delay + ADCH); // This adds Result of Conversion (ADCH) and calls delay function, as "_delay_ms()" requires a constant
				  Led_number = (Led_number + 1);
			  }
			  else
			  {
				  PORTB = (1 << Led_number); // This shifts a 1 along the register in the opposite direction
				  ADCSRA |= 1 << ADSC; // This Starts Conversion
				  delay_ms (Delay + ADCH);
				  Led_number = (Led_number - 1);
			  }
			      UDR0 = ADCH; // This puts the pot value into UDR0 for transmission via serial
			     // UDR0 = '#'; // This puts character '#' into register for transmission

		  }
		  Direction = ! Direction;

    }
}

 void delay_ms( int ms ) // This is a delay function
{
	for (int i = 0; i < ms; i++)
	{
		_delay_ms(1);
	}
}

  

This topic has a solution.
Last Edited: Mon. Nov 4, 2019 - 10:39 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

itoa() ;-)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks,

so I do have to do a conversion then, I just went away and added "Serial.println(ADCH);" to my code and got 0 to 255 on the serial monitor, so the serial print function converts the integer value in ADCH to a string and puts it in a buffer using "iota()" function, and then puts the contents of that buffer into the UDR0 register ? or something along those lines I also have to tell it that the value in the buffer is base 10, also the buffer must be bigger than 8 bits ?. I will have to read some more me thinks,am I getting warmer?

 

cheers

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The buffer in his instance is an array of char. This is standard C stuff, so do some Googling for some online C tutorials.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0


andy.brown100@yahoo.com wrote:
I just went away and added "Serial.println(ADCH);"
Yes but this is C++ and that function is overloaded. You can print a number of different things through the various implementations of .print()/.println().:

    size_t println(const __FlashStringHelper *);
    size_t println(const String &s);
    size_t println(const char[]);
    size_t println(char);
    size_t println(unsigned char, int = DEC);
    size_t println(int, int = DEC);
    size_t println(unsigned int, int = DEC);
    size_t println(long, int = DEC);
    size_t println(unsigned long, int = DEC);
    size_t println(double, int = 2);
    size_t println(const Printable&);
    size_t println(void);

Of those the one you will have been using to print ADCH is:

    size_t println(unsigned char, int = DEC);

The second parameter (which you did not provide, so it defaults) will default to "DEC" so it will be converted to decimal. The actual implementation is then:

size_t Print::println(unsigned char b, int base)
{
  size_t n = print(b, base);
  n += println();
  return n;
}

that invokes:

size_t Print::print(unsigned char b, int base)
{
  return print((unsigned long) b, base);
}

and in turn:

size_t Print::print(long n, int base)
{
  if (base == 0) {
    return write(n);
  } else if (base == 10) {
    if (n < 0) {
      int t = print('-');
      n = -n;
      return printNumber(n, 10) + t;
    }
    return printNumber(n, 10);
  } else {
    return printNumber(n, base);
  }
}

base is 10 so it will take this path:

  } else if (base == 10) {
    if (n < 0) {
      int t = print('-');
      n = -n;
      return printNumber(n, 10) + t;
    }
    return printNumber(n, 10);

So finally it will arrive at:

// Private Methods /////////////////////////////////////////////////////////////

size_t Print::printNumber(unsigned long n, uint8_t base)
{
  char buf[8 * sizeof(long) + 1]; // Assumes 8-bit chars plus zero byte.
  char *str = &buf[sizeof(buf) - 1];

  *str = '\0';

  // prevent crash if called with base == 1
  if (base < 2) base = 10;

  do {
    char c = n % base;
    n /= base;

    *--str = c < 10 ? c + '0' : c + 'A' - 10;
  } while(n);

  return write(str);
}

So that prepares a string (backwards!) by doing repeated division (in this case by 10). So if ADCH had been 123 then it will do 123 % 10 which is 3 and put 3 + '0' into the string then move back one character. It then does (integer) /10 so that drops the 3 and leaves 12, then it does the same to put 2 + '0' into the string and finally the 1 + '0' after which 1/10 is 0 (in integer) so the while() loop ends.

 

The above is a slightly inefficient version of itoa(). In "normal C" you would do something like:

uint8_t n;
char text[20];

n = ADCH;
itoa(text, n, 10);
UART_printstring(text);

Of course this is predicated on a UART_printstring() existing but that's when one might start to analyse the code you have written so far anyway:

			      UDR0 = ADCH; // This puts the pot value into UDR0 for transmission via serial

That is not a "good" way to send serial characters anyway. You aren't checking that the UART is even ready to accept the next character to send - it might still be processing a previous one. You should have something like:

void UART_putchar(char c) {
    // wait until UDRE says the UDR is empty and ready for next char to send
    while(!(UCSR0A & (1 << UDRE0)));
    // only then start the transmission of the next character
    UDR0 = c;
}

The while loop keeps reading UCSR0A and checking for the UDRE bit being set. While it is not set (! is "not") then it remains in this while loop. When the UART signals that UDR is empty and ready for the next byte then 'c' is put into it.

 

So now, wherever you want to send a byte you can just call this UART_putchar('A') or whatever to send a byte.

 

Once you have this routine to call upon the next step is fairly trivial:

void UART_printstring(char * str) {
    while(*str) {
        UART_putchar(*str++);
    }
}

This takes a pointer to characters (usually the address of a char[] array) and the while() checks if the character at the current pointer is 0x00 (which, in C marks the end of strings). If it isn't 0x00 but is '1', '2', '3' of an ADC reading then it does a UART_putchar() of that character and steps 'str' (the pointer to the characters) on one place (str++). That continues until str is pointing at the 0x00 at the end of the string in which case the while() loop ends.

 

So now:

n = ADCH;
itoa(text, n, 10);
UART_printstring(text);

BTW 'n' is not really needed in this - I just used it to separate the steps. The call to itoa() could have been just 

itoa(text, ADCH, 10);

But however it gets there the ADCH reading (which may be a binary value like 123 (0x7B)) is passed into itoa() with the instruction "convert this integer to ASCII in base 10 - that is decimal". itoa=integer to ASCII. As the Arduino code shows this basically involves repeated division by 10 to convert 123 into the component parts: 1, 2, and 3 then '0' is added to each to convert them from binary to ASCII ('3' in ASCII is actually 0x33 not 0x03 so it needs 0x30 added and '0' is another way to write 0x30 because 0x30='0', 0x31='1', 0x32='2' and so on). The output of itoa() is put into the first parameter. That is the [20] byte character array I created - not all bytes used in fact "123" only uses 4 of the 20 bytes (and yes, 4 not 3, because strings always have 0x00 added onto the end). So itoa() takes the 123 in ADCH and converts to '1','2','3',0x00 and then the UART_printstring() routine sends each in turn to the UART until the 0x00 at the end is encountered.

 

As you can see getting 123 from the ADCH reading into a "123" that a human can read (ASCII) is not quite as simple as:

UDR = ADCH;

might first have appeared. In fact if ADCH was 123 (0x7B) then  http://www.asciitable.com/  shows that a 7B passed to a PC terminal program would have printed '{' on the terminal screen:

 

Last Edited: Mon. Oct 28, 2019 - 09:24 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I should add that the code does not wait for the ADC conversion to complete.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thankyou,

 

I shall sit and digest this after work, A quick glance this morning and feel that I can understand this. Now I can study line by line and and truly understand as opposed to copying it "parrot fashion"

I will have a go tonight and let you no how I get on.

 

Thanks for your time 

 

Andy

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

BTW your whole code looks like it could benefit from some modularity. When you have a few lines that look like they "belong together" consider putting that into a function and calling it. For example:

    //***************** ADC0 is used here and is set by default *******************
    ADMUX |= 1 << REFS0; // This sets our reference to the 5v input rail
	ADMUX |= 1 << ADLAR; // This moves the result into the high byte of ADCH for 8 bit
	ADCSRA |= 1 << ADEN; // This Enables the ADC

looks to me like:

void ADC_init() {
    //***************** ADC0 is used here and is set by default *******************
    ADMUX |= 1 << REFS0; // This sets our reference to the 5v input rail
	ADMUX |= 1 << ADLAR; // This moves the result into the high byte of ADCH for 8 bit
	ADCSRA |= 1 << ADEN; // This Enables the ADC    
}

But for the love and honour of God don't go over-board with the |= use. Just use = when you can. There are two lines setting bits in ADMUX there - that could be done in one line with =.

 

Similarly:

	//********* This is setting up USART to send the potentiometer value serialy ***
	UBRR0 = 12; // this sets baud rate at 9600
	UCSR0A = (1 << U2X0); // this doubles data rate in asynchronous mode
	UCSR0B = (1 << TXEN0); // This enables transmit
	UCSR0C = (1 << UCSZ01) | (1 << UCSZ00); // This sets character size to 8 bits

looks to me like:

void UART_init(void) {
	//********* This is setting up USART to send the potentiometer value serialy ***
	UBRR0 = 12; // this sets baud rate at 9600
	UCSR0A = (1 << U2X0); // this doubles data rate in asynchronous mode
	UCSR0B = (1 << TXEN0); // This enables transmit
	UCSR0C = (1 << UCSZ01) | (1 << UCSZ00); // This sets character size to 8 bits    
}

Now your main() starts to looks like:

int main(void)
{
    DDRB = 0xff;

    ADC_init();

    UART_init();

    while (1)
    {

which gives a much clearer "overall picture" of the structure and what's going on here. Similarly:

			  {
				  PORTB = (1 << Led_number); // This shifts a 1 along the register
				  ADCSRA |= 1 << ADSC; // This Starts Conversion
				  delay_ms (Delay + ADCH); // This adds Result of Conversion (ADCH) and calls delay function, as "_delay_ms()" requires a constant
				  Led_number = (Led_number + 1);
			  }
...
			  {
				  PORTB = (1 << Led_number); // This shifts a 1 along the register in the opposite direction
				  ADCSRA |= 1 << ADSC; // This Starts Conversion
				  delay_ms (Delay + ADCH);
				  Led_number = (Led_number - 1);
			  }

these two segments look almost identical apart from the +1 / -1 applied to Led_number at the end so you could easily make a single function for this stuff and just pass in the +1 / =-1 as a parameter:

void readAdc_setLed(int8_t increment)  {
	  PORTB = (1 << Led_number); // This shifts a 1 along the register
	  ADCSRA |= 1 << ADSC; // This Starts Conversion
	  delay_ms (Delay + ADCH); // This adds Result of Conversion (ADCH) and calls delay function, as "_delay_ms()" requires a constant
	  Led_number = (Led_number + increment);
}

But as already noted - the ADC reading is not being done right here. So that itself should be separated into a separate:

uint8_t readADC(void) {
    ADCSRA |= (1 << ADSC);
    while(ADCSRA & (1 << ADSC));
    return ADCH;
}

so the code is more like:

void readAdc_setLed(int8_t increment)  {
      uint8_t reading;
      
	  PORTB = (1 << Led_number); // This shifts a 1 along the register
	  reading = readADC();
	  delay_ms (Delay + reading); // This adds Result of Conversion (ADCH) and calls delay function, as "_delay_ms()" requires a constant
	  Led_number = (Led_number + increment);
}

But all the time keep thinking like this - consider when small/manageable sections of code can be split off into a function. This is especially true when a largely similar piece of code is repeated more than once.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Kartman wrote:
 This is standard C stuff, so do some Googling for some online C tutorials.

Here's some I found earlier:

 

http://blog.antronics.co.uk/2011...

 

(although the focus is on 'C' rather than C++)

 

clawson wrote:
all the time keep thinking like this - consider when small/manageable sections of code can be split off into a function. 

Absolutely!

 

and that is standard programming practice in any language.

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

Thank you for your comments, I haven't yet addressed the problem I originally asked but found your other views very interesting, and have changed the code accordingly, thanks again, I will set about the USART code. I do see what you mean about readability and portability, also less memory used.

I am using Atmel Studio 7 and when I build and download, the code is saved, I would prefer it wasn't as I change things and see what happens a lot, I want to save only when I'm satisfied I have moved forward, this way I no I can always go back to working code, can I change this please?

 

//############### 1Mhz Mouseduino "Knight rider" #################
#define F_CPU 1000000L
#include <avr/io.h>
#include <util/delay.h>

int Led_number;
int Counter;
int Delay = 15;
int Direction = 1;

void delay_ms( int ms );
void init_ADC ();
void init_USART();

int main(void)
{
    DDRB = 0xff;
    init_ADC(); // This sets up analog to digital stuff
    init_USART(); // This sets up serial communication     
    
    while (1) 
    {
            for ( Counter = 1; Counter <= 7; Counter ++) // This is the counter for the loop
            {
                PORTB = 1 << Led_number; // This shifts a 1 along the register
                ADCSRA |= 1 << ADSC; // This Starts Conversion
                delay_ms (Delay + ADCH); // This adds Result of Conversion (ADCH) and calls delay function, as "_delay_ms()" reqires a constant
                Led_number = Led_number + Direction; // This selects which LED to light by shifting left or right dragging zeros behind it (ish)
            }
                Direction = Direction * -1; // This changes the "sign" of direction
    }
}

void delay_ms( int ms ) // This is a delay function
{
    for (int i = 0; i < ms; i++)
    {
        _delay_ms(1);
    }
}

void init_ADC()
{
    ADMUX = ( 1 << REFS0) | ( 1 << ADLAR); // This sets reference to 5v rail and puts the result in high byte of ADC
    ADCSRA = 1 << ADEN; // This Enables the ADC
}

void init_USART()
{
    UBRR0 = 12; // this sets baud rate at 9600
    UCSR0A = 1 << U2X0; // this doubles data rate in asynchronous mode
    UCSR0C = (1 << UCSZ01) | (1 << UCSZ00); // This sets character size to 8 bits
    UCSR0B = 1 << TXEN0; // This enables transmit
}

thanks

 

Andy

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

BTW 'n' is not really needed in this - I just used it to separate the steps. The call to itoa() could have been just 

itoa(text, ADCH, 10);

Hi,

Just a bit confused, should the above read 

iota (ADCH, text, 10);

 

Andy

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The function is itoa - Integer To Ascii

iota is something quite different.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

andy.brown100@yahoo.com wrote:
should the above read 

iota (ADCH, text, 10);

As  Kartman said, it's itoa - not iota - but you are right about the parameter order:

char *  itoa( int value, char * str, int base );

http://www.cplusplus.com/reference/cstdlib/itoa/  

 

EDIT:

 

See #16 & #17 below: this function is not part of the 'C' Standard - so it is entirely implementation-defined !

 

Implementations vary!

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Last Edited: Thu. Oct 31, 2019 - 01:18 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

andy.brown100@yahoo.com wrote:
when I build and download, the code is saved, I would prefer it wasn't

Not quite sure what you mean by that?

 

The source code has to be written to the files on disk, as that's where the compiler will read from to do the compilation!

So, if it didn't save, the compiler would still be compiling the old file contents!

 

The usual way to save "history" is to use a version control system, which lets you retain the history of edits to the project.

 

One simplistic way to do this is to simply take a copy of the entire project tree each time you reach a point worth saving.

On the command line, you can use xcopy; or, in Windows Explorer, you can just drag & drop - and it will create a numbered copy.

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

andy.brown100@yahoo.com wrote:

Just a bit confused, should the above read 

iota (ADCH, text, 10);

Yup, you are absolutely right. User manual is here:

 

https://www.nongnu.org/avr-libc/user-manual/group__avr__stdlib.html#gaa571de9e773dde59b0550a5ca4bd2f00

 

So the first parameter is the number to convert then the second parameter is the address of the character buffer to put the result into.

 

reason for confusion is that there is a far more powerful function like itoa() called sprintf() and in that you really would use:

sprintf(buffer, "%d", ADCH);

so in this case the destination buffer is the FIRST parameter so I mis-remembered itoa() when I wrote:

n = ADCH;
itoa(text, n, 10);

it should have been:

n = ADCH;
itoa(n, text, 10);

This is what happens when you type code here without actually test compiling it.

 

Sorry about the confusion blush

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1


awneil wrote:

As  Kartman said, it's itoa - not iota - but you are right about the parameter order:

char *  itoa( int value, char * str, int base );

http://www.cplusplus.com/reference/cstdlib/itoa/  

Some tool chains have it in different orders:

i.e. Jumpstart C for AVR by ImageCraft

so it will depend on the toolchain used.....   so reference the doc

 

Jim

 

Click Link: Get Free Stock: Retire early! PM for strategy

share.robinhood.com/jamesc3274
get $5 free gold/silver https://www.onegold.com/join/713...

 

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ki0bk wrote:
it will depend on the toolchain used

Oh yes -you're right!

 

The 'C' (and C++) Standard defines atoi(), but not itoa() !

 

surprise

 

It is actually mentioned on the page I referenced:

 

Portability

This function is not defined in ANSI-C and is not part of C++, but is supported by some compilers.

A standard-compliant alternative for some cases may be sprintf:

  • sprintf(str,"%d",value) converts to decimal base.
  • sprintf(str,"%x",value) converts to hexadecimal base.
  • sprintf(str,"%o",value) converts to octal base.

 

I've added a note on my earlier post. 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Last Edited: Thu. Oct 31, 2019 - 01:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
itoa() 

Or, perhaps, utoa(): 

 

https://www.avrfreaks.net/forum/itoa-question

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

awneil wrote:
Or, perhaps, utoa(): 
Makes no difference for a 0..255 value out of ADCH unless there's some suggestion that a reading such as 0x80 is somehow going to be sign extended to 0x8000?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

All is great, thanks to you and all your colleagues.

I am reading through stuff and the fog is clearing! I honestly was a bit lost when it came to strings and data types but I am making progress and I WILL write this bit of code and understand it! This is my hobby, so not as much time as I would like, but I have the number I need in a buffer now just need to feed it to UDR0!. Thanks to you all.

 

Andy

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

What I do in the Arduino IDE is is mess around with the code change things and so on and download to the chip, and if all goes well I save it, but if I get in a pickle I can close without saving and the original I started with that evening is untouched, so start again! not the right way but a habit I must break. With studio 7 I change something and an asterisk appears (on the tab)  to say it's changed but when I upload to the chip to see what happens the changes are made to the  original so no going back....bet this makes no sense at all, put basically I don't want the original software touched until i'm done for the night. 

 

Thanks 

Andy

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

andy.brown100@yahoo.com wrote:
and if all goes well I save it, but if I get in a pickle I can close without saving and the original I started with that evening is untouched,
you might want to explore a revision control system like SVN or Git. It makes "trying out ideas" then "winding back" trivial and what's more you get to keep all your experiments so if you later think "if only I'd known this when I was trying X, Y or Z it would probably have worked" then you can dig X/Y/Z out if the history, merge it onto what you've got  now and carry on the experiment.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

awneil wrote:

clawson wrote:
all the time keep thinking like this - consider when small/manageable sections of code can be split off into a function. 

Absolutely!

and that is standard programming practice in any language.

 

Unless your compiler is smart enough (or was instructed to) inline them, there is a performance penalty for calling a function and returning therefrom.  It's small, but worth mentioning.  S.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

Thank you for your help, I have a crude but working solution and I am gonna move on to interrupts and timers next, maybe I can think through it without any help, which is what I try to do, but I no if I hit a mental block I can come here for help and not be treated as a fool (bad experience with another forum!)

I have one final question, after the last character is outputted I place a space (ascii 32) but I would like a carriage return instead so I thought simply pass a 13, but not so simple, is there an easy answer to this ?

Thanks to all who helped.

 

Andy.

 

//############### 1Mhz Mouseduino "Knight rider" #################
#define F_CPU 1000000L
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>

int Led_number;
int Counter;
int Delay = 15;
int Direction = 1;
char pot [4];
int i = 0;
void delay_ms( int ms );
void init_ADC ();
void init_USART();
void serial_send();

int main(void)
{
    DDRB = 0xff; // This sets port B to outputs
    init_ADC(); // This sets up analog to digital stuff
	init_USART(); // This sets up serial communication 	

    while (1)
    {
			for ( Counter = 1; Counter <= 7; Counter ++) // This is the counter for the loop
			{
				PORTB = 1 << Led_number; // This shifts a 1 along the register
				ADCSRA |= 1 << ADSC; // This Starts Conversion
				delay_ms (Delay + ADCH); // This adds Result of Conversion (ADCH) and calls delay function, as "_delay_ms()" reqires a constant
				Led_number = Led_number + Direction; // This selects which LED to light by shifting left or right dragging zeros behind it (ish)
				serial_send();
			}
		        Direction = Direction * -1; // This changes the "sign" of direction
    }
}

void delay_ms( int ms ) // This is a delay function
{
	for (int i = 0; i < ms; i++)
	{
		_delay_ms(1);
	}
}
void init_ADC() // This initializes the analog to digital converter
{
	ADMUX = ( 1 << REFS0) | ( 1 << ADLAR); // This sets reference to 5v rail and puts the result in high byte of ADC
	ADCSRA = 1 << ADEN; // This Enables the ADC
}
void init_USART() // This initializes the serial communication
{
	UBRR0 = 12; // this sets baud rate at 9600
	UCSR0A = 1 << U2X0; // this doubles data rate in asynchronous mode
	UCSR0C = (1 << UCSZ01) | (1 << UCSZ00); // This sets character size to 8 bits
	UCSR0B = 1 << TXEN0; // This enables transmit
}
void serial_send() // This converts the pot reading to a "char" and sends it to serial monitor
{
	for ( i = 0; i <= 2; i++)
	{
		while (!(UCSR0A & (1 << UDRE0))); // This will loop until UDR0 is ready to handle data
		itoa(ADCH, pot, 10); // This converts the pot value in the ADC to a character
		UDR0 = pot[i]; // This places each character of the pot value on the serial line one byte at a time
	}
	while (!(UCSR0A & (1 << UDRE0)));
	UDR0 = 32; // This places a space after the 3 digit pot value
	i = 0; // This resets loop
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

One function - one job. Your serial_send does too many things.

 

It also has critical errors - itoa() returns a variable length string based on the value you give to it. There's nothing to say it will always be 3 chars. The string is terminated by a value of 0. You also call itoa() each time in your loop. This may have bizarre side effects as ADCH may change.

 

Write a function to send a char to the usart - don't keep repeating the same code over and over.

 

void uart_putchar(char c)
{
   while (!(UCSR0A & (1 << UDRE0)));
   UDR0 = c;
}

so you can simply write:

uart_putchar(pot[i]);
uart_putchar(' '); //print a space 
uart_putchar(13); //carriage return or 
uart_putchar(10); //line feed

 

Have a read of this:

 

https://www.avrfreaks.net/forum/...

 

sending the adc value out the usart will boil down to this:

 

printf("%d\r\n",ADCH); //with carriage return \r and line feed \n (depends what your terminal program expects

 

This is standard C stuff. You don't want to re-invent the wheel.

 

 

Last Edited: Mon. Nov 4, 2019 - 11:40 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
	for ( i = 0; i <= 2; i++)
	{
		while (!(UCSR0A & (1 << UDRE0))); // This will loop until UDR0 is ready to handle data
		itoa(ADCH, pot, 10); // This converts the pot value in the ADC to a character
		UDR0 = pot[i]; // This places each character of the pot value on the serial line one byte at a time
	}

This is just plain weird. Why are you doing the itoa() inside the loop that is then consuming the converted characters ?? Surely you do:

    itoa(ADCH, pot, 10); // This converts the pot value in the ADC to a character
    for ( i = 0; i <= 2; i++)
    {
        while (!(UCSR0A & (1 << UDRE0))); // This will loop until UDR0 is ready to handle data
        UDR0 = pot[i]; // This places each character of the pot value on the serial line one byte at a time
    }

except that you wouldn't do this (see Kartiman's comments in #25) so it would look more like:

    itoa(ADCH, pot, 10); // This converts the pot value in the ADC to a character
    for ( i = 0; i <= 2; i++)
    {
        uart_putchar(pot[i]); // This places each character of the pot value on the serial line one byte at a time
    }

except you wouldn't even do that as you will almost certainly have a uart_putstring() to match uart_putchar() so it would actually be:

    itoa(ADCH, pot, 10); // This converts the pot value in the ADC to a character
    uart_putstring(pot);

Except that, as Kartman says, the real solution is FDEV_SETUP_STREAM so this all becomes:

printf("%d\r\n",ADCH);

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

andy.brown100@yahoo.com wrote:

I have one final question, after the last character is outputted I place a space (ascii 32) but I would like a carriage return instead so I thought simply pass a 13, but not so simple, is there an easy answer to this ?

 

Check your terminal settings.  See if it will put a line feed (LF) after a carriage return (CR).  S.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Scroungre wrote:
Check your terminal settings

+1

 

eg, see: https://www.avrfreaks.net/commen...

 

 

andy.brown100@yahoo.com wrote:

 simply pass a 13,

Avoid using "Magic Numbers" like that.

 

The 'C' programming language gives you '\r' for CR and '\n' for LF (newline).

 

 

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

Firstly thanks to all, and yes putting "itoa" in the loop was pretty stupid I get that now, and also I see how having a function to do one job is the idea, and now I have a simple function call to output a single character to the UDR0 register, but in the future I am going to need to output whole strings. You guys (gals?) have nudged me through this and I'm grateful. This is how it looks now. When I send a carriage return the output scrolls diagonally but Scroungre and awneil suggested my terminal settings, and this makes sense as in the arduino serial monitor it does what i expected. This code has no end for me now I will keep adding things and learn as I go (wish i had you lot on tap!) next thing I will do is get the output to a small screen probably a 0.96" Oled and begin towards writing Doom for the 328p_AU.

//############### 1Mhz Mouseduino "Knight rider" #################
#define F_CPU 1000000L
#include <avr/io.h>
#include <util/delay.h>
#include <stdlib.h>

int Led_number;
int Counter;
int Delay = 15;
int Direction = 1;
char pot_value [4];

void delay_ms( int ms );
void init_ADC ();
void init_USART();
void serial_send(char val);

int main(void)
{
	DDRB = 0xff; // This sets port B to outputs
	init_ADC(); // This sets up analog to digital stuff
	init_USART(); // This sets up serial communication

	while (1)
	{
		for ( Counter = 1; Counter <= 7; Counter ++) // This is the counter for the loop
		{
			PORTB = 1 << Led_number; // This shifts a 1 along the register
			ADCSRA |= 1 << ADSC; // This Starts Conversion
			delay_ms (Delay + ADCH); // This adds Result of Conversion (ADCH) and calls delay function, as "_delay_ms()" reqires a constant
			Led_number = Led_number + Direction; // This selects which LED to light by shifting left or right dragging zeros behind it (ish)
			itoa(ADCH, pot_value, 10); // This converts the pot value in the ADC to a character
			for (int i = 0; i <= 2; i++)
			{
				serial_send (pot_value[i]);
			}
			serial_send (' ');
		}
		Direction = Direction * -1; // This changes the "sign" of direction
	}
}

void delay_ms( int ms ) // This is a delay function
{
	for (int i = 0; i < ms; i++)
	{
		_delay_ms(1);
	}
}
void init_ADC() // This initializes the analog to digital converter
{
	ADMUX = ( 1 << REFS0) | ( 1 << ADLAR); // This sets reference to 5v rail and puts the result in high byte of ADC
	ADCSRA = 1 << ADEN; // This Enables the ADC
}
void init_USART() // This initializes the serial communication
{
	UBRR0 = 12; // this sets baud rate at 9600
	UCSR0A = 1 << U2X0; // this doubles data rate in asynchronous mode
	UCSR0C = (1 << UCSZ01) | (1 << UCSZ00); // This sets character size to 8 bits
	UCSR0B = 1 << TXEN0; // This enables transmit
}
void serial_send(char val) // This converts the pot reading to a "char" and sends it to serial monitor
{
	while (!(UCSR0A & (1 << UDRE0))); // This will loop until UDR0 is ready to handle data
	UDR0 = val ; // This places a character on the serial line one byte at a time
}

Thanks 

Andy

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

andy.brown100@yahoo.com wrote:

			ADCSRA |= 1 << ADSC; // This Starts Conversion
			delay_ms (Delay + ADCH); // This adds Result of Conversion (ADCH) and calls delay function, as "_delay_ms()" reqires a constant

This still looks wrong - you appear to be using ADCh (the reading) immediately after setting the start conversion bit. There's nothing here to wait for that conversion to complete before using ADCH - so how do you know it is valid?

 

Also:

			itoa(ADCH, pot_value, 10); // This converts the pot value in the ADC to a character
			for (int i = 0; i <= 2; i++)
			{
				serial_send (pot_value[i]);
			}

Again you don't seem to have grasped what we've been suggesting. The plan was that in addition to:

void serial_send(char val) // This converts the pot reading to a "char" and sends it to serial monitor
{
	while (!(UCSR0A & (1 << UDRE0))); // This will loop until UDR0 is ready to handle data
	UDR0 = val ; // This places a character on the serial line one byte at a time
}

you would then have:

void serial_send_string(char * str)
{
    while(*str) {
        serial_send(*str++);
    }
}

Now you can use:

            itoa(ADCH, pot_value, 10); // This converts the pot value in the ADC to a character
            strcat(pot_value, ' ');
            serial_send_string(pot_value);

You don't need this stuff any more:

			for (int i = 0; i <= 2; i++)
			{
				serial_send (pot_value[i]);
			}
			serial_send (' ');

(Anyway Kartman already told you that you can't use a fixed number in a for() loop anyway, suppose the ADCH reading were 0 or 3 or 17 or 91 or 137 or 255. If the reading were 3 for example (so "3" after the itoa) how would the "i <= 2" bit get on do you think?)

 

Oh and if you wonder what all the whil(*str) is all about - just read Freaks, it's been explained about a million times.

Last Edited: Mon. Nov 4, 2019 - 05:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

andy.brown100@yahoo.com wrote:
When I send a carriage return (sic?) the output scrolls diagonally

You mean something like this:

 

If that happens, it's because you're sending LF (line-feed) - not CR (carriage return).

 

https://superuser.com/questions/654490/putty-new-line-not-working-properly

 

 

EDIT

 

remove excess quote

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Last Edited: Mon. Nov 4, 2019 - 05:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

His code is not attempting to send either '\n' nor 'r' in fact. Just a space after the reading. So I rather think the diagonal is more like:

107 213 119 99  146
 241 101 205 182 13
2 155 135 222 207 1
44 etc.

which suggests it's simply that the width of each reading is not an exact divisor of the terminal width.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But he said it happens when he does send "carriage return" ?

 

Andy Brown - please clarify !

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Nothing in the code in #29 is sending '\n' or '\r' ??

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

maybe he tried it, found it didn't work to his satisfaction, and took it out again?

 

I don't know - it's what he said:

In #27, andy.brown100@yahoo.com wrote:
When I send a carriage return the output scrolls diagonally

 

So we need him to clarify what he meant by that!

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
strcat(pot_value, ' ');

char pot_value[4];

itoa( ADCH, pot_value, 10 ); //pot_value -> { '2', '5' , '5', 0 }

strcat( pot_value, " " );        //pot_value -> { '2', '5' , '5', '  ' }   -- > 0 <-- where am I now? (into some other global var)

(strcat needs a const char*, so " " instead of a char ' ')

 

I would suggest making formatting separate unless using printf/snprintf type things. Make the the pot_value buffer do a single job- hold up to a 3 digit decimal number with 0 terminator. You could expand its size, but now you have to be careful because you may decide to add more, and a problem may not be discovered until long after you decided everything is working ok. The above code is probably a good example- may work fine, but go above 99 and you have a problem, and it may not be obvious for quite a while when it starts overwriting some neighboring variable. 'Once in a while' type problems are buggers to chase down.

 

serial_send_string( pot_value );

serial_send_string( "\r\n" ); or serial_send( ' ' ); or whatever

 

Whether its one call or more, makes no difference, you will be waiting on the uart in either case. Pick whatever is easiest to maintain/read/change.

 

I tend to send '\r\n' for line endings- the worst that happens is I get double spaced lines. Telling a terminal to just follow orders and don't mess with cr/lf is sometimes easier than trying to come up with the right combo of settings to match whatever was decided on for a line ending.

Last Edited: Tue. Nov 5, 2019 - 06:44 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi all,

Sorry been away, now to get on, and yes this is what it's doing, will try to fix this and address the recommendations by the other guys.

Not ideal I'm sure, but a carriage return took me back to the beginning of same line so I just sent a form feed first, so I sent to my serial function the ASCII values 12 13 and got the desired result. 

 

thanks all

Andy.

Last Edited: Wed. Nov 6, 2019 - 05:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

andy.brown100@yahoo.com wrote:
so I just sent a form feed first
I trust you mean LINE feed? A Form feed is a whole new kettle of worms !

 

Bottom line: some receivers will take just "\n" to move to the start of the next line, some need "\r\n". Often it's a Windows versus Linux thing. (there may be some esoteric systems/terminals that require something other than "\n" or "\r\n" but they will be very few and far between)

 

Personally (probably because I spent so much of my life in a Linux world) I only ever send "\n". if the receiver does not then behave as I hope I use its own settings to make it do so (most terminals have definable action for \n and \r)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

andy.brown100@yahoo.com wrote:
a carriage return took me back to the beginning of same line

Yes, of course it did: exactly as the name "Carriage Return" suggests it should!

 

"Carriage Return" will return the "carriage" (ie, the position at which the next print will occur) to the beginning of the line.

 

"Line Feed" feeds the "paper" forward by 1 line; ie, it sets the position at which the next print will occur to the same column as current, but on  the next line.

 

So, to set the print position to the beginning (first column) of the next line, you need both CR and LF.

 

But, as noted in #28, most terminals can be configured to add an implicit CR after LF (and various other options).

 

As clawson said, a Form Feed (FF, 0x0C) is something different again.

 

You still haven't explained what you meant by "diagonal" scrolling.

 

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Personally (probably because I spent so much of my life in a Linux world) I only ever send "\n".

Windows uses explicit CRLF - which is probably why I tend to use both!

 

if the receiver does not then behave as I hope I use its own settings to make it do so (most terminals have definable action for \n and \r)

Indeed:

See #28, which links to https://www.avrfreaks.net/commen...

 

 

I guess using just LF is "safer", as it's easier to have the terminal implicitly add the line-feed if required than it would be to discard an unnecessary CR ... ?

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,Yes, I'm just sending a "line feed" ASCII 10 and then telling Terra Term that, as opposed to changing my code or should I be "safe" and send the carriage return anyway, this is my first use of a terminal like this so which is better in the real world ? After thinking about it it's probably better to include the CR, as this way when I open PUTTY it works as desired straight away and also works the same in Tera Term. Now back to some other points raised.

 

Thanks 

Andy

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0


andy.brown100@yahoo.com wrote:
include the CR, as this way when I open PUTTY it works as desired straight away and also works the same in Tera Term.

You've missed the point!

 

The behaviour is configurable - so it doesn't depend on what application you have; it depends on how you have it configured!

 

Both PuTTY and TeraTerm can be configured:

 

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

My reasoning was to leave the code in there because I didn't want to change the settings every time, I new I could but it was a hassle  , but I now see that PuTTY has a "save sessions" option so that's not a problem, I'm gonna take the two commands out. Thanks for your thoughts.

 

Andy

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You could have a function to send a new line, and use the function whenever you want to send a 'new line'

void uart_send_nl(void)
{
#if 1
    uart_send('\r');
#endif
    uart_send('\n');
}

Then you only have one place to change in your code if you want to change the behaviour.