Strange code behaviour

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

Hi all. I'm fairly new to the AVR world so please bare with me. I've spent a long time looking for solutions to my problem, which has resulted in me spotting various things wrong with my code which I've fixed, but alas it's still not quite right. I really don't know what's going on so it's very difficult to search for an answer to an undefined problem!

 

Basically I'm writing what should be a simple program to read the output from a rotary encoder, increment/decrement a counter and display the counter on a LCD screen. I'm using Atmel Studio and an ATTiny2313. This forms part of a wider project I'm working on but that's not relevant. For the LCD display I'm using a HD77480 library that I found here:

 

http://homepage.hispeed.ch/peter...

 

I've used this before in other code and it's worked fine.

 

For the rotary encoder part, I've written a function myself, which is most likely causing trouble but I just can't see how. The function reads the current phase from the encoder and compares against the previous phase in order to determine CW or CCW motion. The comments explain a bit more, but essentially if the new phase is a valid move it will (should) update the enc_status variable to indicate a CW or CCW move, if not a valid move it should simply ignore it and exit the function. I'm sure it could be written far more efficiently but hey, I'm just getting started so this will do for me. The function returns enc_status that is either 0 (no change), 1 (CW motion) or -1 (CCW motion).

 

The main function then calls this function and increments/decrements a variable depending on the result.  This variable of type int is then converted to a character string, using itoa(), in order to be passed to the lcd_puts() function in the LCD library that writes to the LCD display. I've used this very code (different variable names) previously and it has worked fine.

 

My issue is that there seems to be some interference between the LCD library functions and either the itoa() function and/or my encoder function. The reason I suspect this is because I'm seeing some strange results on the LCD display. As you can see in my code I have the LCD set to display "Counter:" on the first line and simply "0" on the second line. However after programming to the MCU the LCD screen initially just displays a "-1" on the bottom line. And then if I start to rotate the encoder the number on the LCD increases and decreases, even though the encoder variable isn't being sent there! It should just read "Counter:" and "0". So clearly somehow the encoder variable is somehow getting to the LCD, but I have no idea how. The number also doesn't behave how it should (if it were programmed to display!). It increments and decrements in 10's not 1's and once it goes to double figures for some reason the least significant digit appears again in the top line.

 

If I comment out the line that calls my encoder function, the LCD screen displays what I've put in the lcd_puts() functions. If I comment out the itoa() line and the line above, it also displays correctly.

 

Having just power cycled it again (for the nth time) this time the "Counter:" and "0" indeed did appear initially! And then the number would again increment/decrement according to the encoder. After another power cycle it's back to just displaying "-1".  It has to be something to do with the function I've written. With the exact same hardware I've had the LCD screen behaving perfectly in the past, but with the addition of the encoder it's all gone hay wire.

 

Code attached. Very confused. Any help would be greatly appreciated.

 

Cheers!

Attachment(s): 

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

First somewhere in this forum there are code that work the way you try to do it.

Make sure that your code don't count up(or down) when channel A is constant and B go up and down, it should go one up and then one down .....

A way I have solved it is by having a 4 bit LUT with A and B from last time and the value now. and the LUT holds the value -1, 0 ,1 so you just add the value to your encoder position.

and then you call the code from either a timer ISR, or when A or B has changed.

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

At the very least this code is broken:

 

char *char_count = "0"; // Define character string to be sent to LCD and assign arbitrary content
itoa (count,char_count,10);

You must not modify what char_count points to. If you want an array to modify, do this:

char char_count [8];

As others have said, your rotary decode's verbose implementation is inviting subtle bugs. If I were you, i'd first replace that with something that incremented every second, to be sure that the LCD output code was working first. Debugging two problems at once is difficult.

Last Edited: Fri. Oct 24, 2014 - 10:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

blargg wrote:
Debugging two problems at once is difficult.

+99!

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

breakshift wrote:
I've used this very code (different variable names) previously and it has worked fine.

Just because you haven't observed any problems yet doesn't mean that there aren't any.

 

I'm sure we've all been in the position where we look back at a piece of code and think, "how did that ever work??"

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

blargg wrote:

At the very least this code is broken:

 

char *char_count = "0"; // Define character string to be sent to LCD and assign arbitrary content
itoa (count,char_count,10);

You must not modify what char_count points to. If you want an array to modify, do this:

char char_count [8];

 

Thanks. I've changed it from a pointer to an array as you suggested and I'm getting much better results. Having made that change I could then see more clearly that the LCD was not clearing zero's from the screen as I increased and decreased the counter. By that I mean if I went over 100 and then back down below, it would leave the a zero in the third space instead of clearing it. So I just added a line to write 8 spaces to the bottom line of the LCD before I write the counter value. That's cleared it all up, and it's working correctly now.

 

Thanks for the help everyone. Simple problem, simple solution!

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

breakshift wrote:
the LCD was not clearing zeros from the screen

Well, the LCD only does what it's told - so it was really the code that wan't clearing those zeros.

 

This could well be an example of a bug that's always been there, but never happened to get noticed...

 

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:

Well, the LCD only does what it's told - so it was really the code that wan't clearing those zeros.

 

Yes I agree, but the character array I'm sending to the LCD is 8 characters long. So I thought that any blank spaces in the array would manifest in the display as a space/blank. Since the array is written to via the itoa() function, it must only write the actual counter figures and leave the remaining spaces in the array as just vacant/blank, which do not manifest in the LCD display as blank spaces. So the LCD display function by the looks of it only takes the characters in the array that are actually defined, and forgets about the blanks, and so the LCD itself just continues to display the last character it received for that space, which would be a 0.

 

So yes either way it's still my code not the LCD itself. But you get what I mean. 

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

itoa() only writes the minimal number of characters.    i.e left-adjusted.   There will be no spaces.

 

Use sprintf() if you want convenient formatting.    Or do your own padding with itoa().

 

David.

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

There will be no spaces.

 In addition, OP should read how itoa() terminates the output string.  And think about if an 8-character output string and a terminator fit into an 8 byte array.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

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

breakshift wrote:

awneil wrote:

 

Well, the LCD only does what it's told - so it was really the code that wan't clearing those zeros.

 

Yes I agree, but the character array I'm sending to the LCD is 8 characters long. So I thought that any blank spaces in the array would manifest in the display as a space/blank. Since the array is written to via the itoa() function, it must only write the actual counter figures and leave the remaining spaces in the array as just vacant/blank, which do not manifest in the LCD display as blank spaces.

 

Expanding on what others have already responded with... It's fortunate that you detailed your thinking on the matter, because it doesn't match how things actually behave. That 8-character array just has the capacity for a 7-character string (plus the terminator, making 8 bytes). C-style strings have a dynamic length (varies during program execution). This array's length is always 8 characters, unvarying. The length of the string is whatever you've put into the array. Before itoa(), the array has no reliably-valid string in it; attempts to even use it might result in undefined behavior. itoa() only generates a string of the necessary length to hold the decimal representation of the value passed. So your LCD string printing routine normally prints fewer than 7 characters to the LCD.

 

breakshift wrote:
So the LCD display function by the looks of it only takes the characters in the array that are actually defined, and forgets about the blanks, and so the LCD itself just continues to display the last character it received for that space, which would be a 0.

 

Look at your lcd_puts(): the loop stops once it finds the null terminator. How would it even know that you're passing it an 8-character array? So it's not that itoa() puts some special characters to pad the string to use all 8 bytes of the array (as with lcd_puts(), how would itoa() know how long your array is?), it's that it terminates the string so that there's nothing defined past it.