ATMega328p registers behaving differently in sketch and flashed firmware?

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

Hello everyone,

I am hitting my head against a brick wall for the past hours here and was wondering if I was slowly going insane.
I have a 4x4 Keypad attached to an Arduino Uno R3 and am getting mixed results reading the pins.

I have tested the following code in a Sketch and uploaded it to the board and it works wonderfully. (Modified snippet from the keypad library to not use the arduino framework since I need it lightweight outside of the arduino framework...)
However, if I compile this down from a c project into a hex and flash the bootloader, I get 0 results back from the keypad.

 

    
... header ...
#include <avr/io.h>

#define F_CPU 16000000
#define COLS 4 
#define ROWS 4
 
#define COL(N) (1 << (N+4))
#define ROW(N) (1 << N)

#define COL_REG DDRD
#define ROW_REG DDRB

#define COL_PORT PORTD
#define ROW_PORT PORTB

#define COL_PIN PIND
#define ROW_PIN PINB

... snip to .c file, left out function header, called within while loop...    
    // pull up pins
    for (int i = 0; i < ROWS; i++)
    {
        ROW_PORT |= ~ROW(i);
        ROW_REG &= ~ROW(i);
    }

    for (int i = 0; i < COLS; i++)
    {
        COL_REG |= COL(i);  // set col to output
        COL_PIN &= ~COL(i); // set col to LOW
        for (int j = 0; j < ROWS; j++)
        {
            if (!(ROW_PIN & ROW(j)))
            {
                curKey = hexKeys[j][i];
                keyPressFound = 1;
            }
        }
        COL_PIN |= COL(i);  // set pin to high, ending pulse
        COL_REG &= ~COL(i); // Set col back to input
    }
... snip ...

 

The wiring of the keypad goes from pin 4 to 11, but the wiring should be ok considering it works inside a sketch.

My question is: is there any reason why the registers should behave differently in an uploaded sketch as opposed to a flashed hex? I know that arduino uses the pinMode and digitalWrite methods, which resolve pin->register relations based on the pin layout header files, but if this resolution was wrong on my part (adapted this from here: https://www.arduino.cc/en/Reference/PortManipulation ), this should also fail inside a sketch, unless it resolves it differently based on whether it's a sketch or a flashed firmware?

Thanks in advance and kind regards

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

I'm not sure why, but I wasn't able to get scan as fast as waiting just one instruction cycle and read it back, not even few of them. I had to add hunderds of microseconds delays to make it work. I even tried to discharge input lines by enabling outputs and so on.

 

Later I tried to do it with STM32 and worked flawless even without any significant delays (maybe some nop instruction)

 

So you can start with short delays to see if it's not just too fast.

Computers don't make errors - What they do they do on purpose.

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

However using those macros hides the fact you are writing into a wrong port. Or you can just use  PIND = COL(i)  toggle pin to low and then the same to togle it to high.

    for (int i = 0; i < COLS; i++)
    {
        DDRD |= COL(i);
        PIND &= ~COL(i); // toggles who know what (but definitely not COL(i)
        for (int j = 0; j < ROWS; j++)
        {
            if (!(ROW_PIN & ROW(j)))
            {
                curKey = hexKeys[j][i];
                keyPressFound = 1;
            }
        }
        PIND |= COL(i);  // toggle COL(i)
        DDRD &= ~COL(i);
    }

 

Computers don't make errors - What they do they do on purpose.

Last Edited: Sat. Dec 21, 2019 - 08:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You need some kind of delay and debouncing.   Key presses are about 0.2 seconds long with about 0.4 second intervals between different keypresses.  Your scanning code is running thousands of iterations with each key press/release.    Test the entire keypad for a pressed key and store the result.  Test again after about 300 milliseconds to see if the new result is the same as the previous result.

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

Hello everyone, thank you for your answers!

KIIV_cz wrote:

I'm not sure why, but I wasn't able to get scan as fast as waiting just one instruction cycle and read it back, not even few of them. I had to add hunderds of microseconds delays to make it work. I even tried to discharge input lines by enabling outputs and so on.

 

Later I tried to do it with STM32 and worked flawless even without any significant delays (maybe some nop instruction)

 

So you can start with short delays to see if it's not just too fast.

That was a good idea, however even if running the keyscan only every 200ms it still detects no input whatsoever. I have also added nop's after setting the columns to output and in various other places, de-factor bruteforcing any possible combinations of delays. Nothing helped.

KIIV_cz wrote:

However using those macros hides the fact you are writing into a wrong port. Or you can just use  PIND = COL(i)  toggle pin to low and then the same to togle it to high.

    for (int i = 0; i < COLS; i++)
    {
        DDRD |= COL(i);
        PIND &= ~COL(i); // toggles who know what (but definitely not COL(i)
        for (int j = 0; j < ROWS; j++)
        {
            if (!(ROW_PIN & ROW(j)))
            {
                curKey = hexKeys[j][i];
                keyPressFound = 1;
            }
        }
        PIND |= COL(i);  // toggle COL(i)
        DDRD &= ~COL(i);
    }

 

 

PIND &= ~COL(i) unsets the bit at PIND. This is a procedure I took from the arduino keypad library, but through testing I have noticed that it isn't even necessary. The keypad works just fine within a sketch without that line. Sadly still no luck on a flashed bootloader.

Simonetta wrote:

You need some kind of delay and debouncing.   Key presses are about 0.2 seconds long with about 0.4 second intervals between different keypresses.  Your scanning code is running thousands of iterations with each key press/release.    Test the entire keypad for a pressed key and store the result.  Test again after about 300 milliseconds to see if the new result is the same as the previous result.

I have added code that only scans every Nms (have tried everything from 40 to 400), sadly still no luck getting anything back from the pins.

I was considering that maybe I've got my registers wrong from the avr/io.h.

In my makefile I define:
 

MCU          = atmega16u2
ARCH         = AVR8
F_CPU        = 16000000
F_USB        = $(F_CPU)

which should be correct for an r3 uno? In any case it's the only mcu that FLIP will load when connecting via USB, so I'm assuming that is correct. 
There is another curiosity here though, I tried initializing UART, however it seems that avr/io.h does not define UBRRH / UBRR0H or UCSRB / UCSR0B for me, despite compiling for atmega16u2 which, if i didn't read the documentation wrong, should have these registers? I am having a creeping suspicion here that perhaps avr/io.h is being compiled with a wrong flag and my register addresses are simply not correct, leading to faulty reads when self compiling a firmware.

However the compiler output seems to be catching the correct flag

 

' [GCC]     :' Compiling C file \"Project.c\"
avr-gcc -c -pipe -gdwarf-2 -g2 -mmcu=atmega16u2 -fshort-enums -fno-inline-small-functions -fpack-struct -Wall -fno-strict-aliasing -funsigned-char -funsigned-bitfields -ffunction-sections -I. -DARCH=ARCH_AVR8 -DF_CPU=16000000UL -mrelax -fno-jump-tables -x c -Os -std=gnu99 -Wstrict-prototypes -DUSE_LUFA_CONFIG_HEADER -IConfig/  -I. -I./lufa/LUFA/.. -DARCH=ARCH_AVR8 -DBOARD=BOARD_NONE -DF_USB=16000000UL  -MMD -MP -MF obj/Project.d Project.c -o obj/Project.o

 

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

You didn't comment your intention properly for these two lines:

... snip to .c file, left out function header, called within while loop...    
    // pull up pins
    for (int i = 0; i < ROWS; i++)
    {
        ROW_PORT |= ~ROW(i);
        ROW_REG &= ~ROW(i);
    }

A first glance the two bit inverts looks wrong - but only you can verify that.

 

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

Nonsense.

 

The PINx register is used for reading real state at the input pins. And as a bonus feature it can be used for toggling PORTx bits by writing logical 1 to corresponding bits of PINx.

 

 

That means, they either uses  PORTx |= bitmask; and PORTx &= ~bitmask;   or   PINx = bitmask; (it must be in correct state before it starts)

 

EDIT:

	virtual void pin_write(byte pinNum, boolean level) { digitalWrite(pinNum, level); }
	virtual int  pin_read(byte pinNum) { return digitalRead(pinNum); }

The digitalWrite definitely doesn't mess with PINx register. That's what digitalRead do.

 

Computers don't make errors - What they do they do on purpose.

Last Edited: Sun. Dec 22, 2019 - 01:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

KIIV_cz wrote:
The PINx register is used for reading real state at the input pins. And as a bonus feature it can be used for toggling PORTx bits by writing logical 1 to corresponding bits of PINx.

Indeed I misread that part of the documentation, thank you for pointing that out to me!  
As mentioned above however I had already tried not using PINx and instead using PORTx, and removing them altogether. 

The result is the same, it works perfectly when a sketch is uploaded to the board, but not with a flashed bootloader.

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

N.Winterbottom wrote:

You didn't comment your intention properly for these two lines:

... snip to .c file, left out function header, called within while loop...
    // pull up pins
    for (int i = 0; i < ROWS; i++)
    {
        ROW_PORT |= ~ROW(i);
        ROW_REG &= ~ROW(i);
    }

A first glance the two bit inverts looks wrong - but only you can verify that.

I agree, looks wrong.

My guess is you are intending to configure pin as input with pullup enabled.

In which case you would want ROW_PORT |= ROW(i), pretty much the opposite of what you have.

However, as long as you go round the loop at least 2 times, you end up with all bits in ROW_PORT set to 1, so it probably works, but for the wrong reason.

 

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

MrKendo wrote:

N.Winterbottom wrote:

You didn't comment your intention properly for these two lines:

... snip to .c file, left out function header, called within while loop...
    // pull up pins
    for (int i = 0; i < ROWS; i++)
    {
        ROW_PORT |= ~ROW(i);
        ROW_REG &= ~ROW(i);
    }

A first glance the two bit inverts looks wrong - but only you can verify that.

I agree, looks wrong.

My guess is you are intending to configure pin as input with pullup enabled.

In which case you would want ROW_PORT |= ROW(i), pretty much the opposite of what you have.

However, as long as you go round the loop at least 2 times, you end up with all bits in ROW_PORT set to 1, so it probably works, but for the wrong reason.

 

 

Oh damn, you are absolutely right. Late night typo on my part. Thank you! Sadly that didn't help the non functional keypad once it's on the bootloader. 

I have ordered a ATMega32u4 board to test this out tomorrow, since I compile and flash for ATmega16u2 with FLIP but the actual board on the UNO -r3 is a 328 (and the 16u2 only handles the usb and is a secondary chip on the UNO, if I am correct?) I think that the header files might just have the wrong registers or flashing the 16u2 chip does not allow me to read/manipulate the registers that are meant for the 328 chip.. It would also explain why I can't seem to get UART to work, since the registers specified for the 328 do not exist in the avr/io.h for the 16u2.

 

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

What is your actual target? A mega 16u2 or a mega328? Title says 328 but your posts and files mention the 16u2

Which is it?

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

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

I have switched to a atmega32u4 chip (leonardo) which doesn't have 2 chips in one. So the target now is a 32u4. I have changed all the pins to respect the new pinout and again, works flawlessly inside a sketch uploaded from the arduino ide. 
However if I flash the same code from a compiled hex, compiling the avr headers for atmega32u4 as a target I still get nothing.
 

Here's a thought, I am using the chip as a faux USB device using the LUFA library. Could that be interfering with the way the digital pins work?

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

Just because a device is in the AVR family does not mean that you can write code(or use code pre-written) for one device and expect it to work on a completely different device.

 

You will need to adjust registers accordingly.

 

What is the desired function of your widget?  Does it need USB?  If it does then you have to set up a USB driver for the AVR >>LUFA for example<<  If you do not need usb then the Mega328 should be able to do what you need. 

 

If you are a beginner with all of this and you need USB, I would strongly suggest you stick with the Mega328 and use an external USB part.  Like the ones from FTDI, Silicon Labs, or the CH340.

 

 

Write down on paper what you want to achieve, and post it here.  Then lets set this up properly.

JIm

I would rather attempt something great and fail, than attempt nothing and succeed - Fortune Cookie

 

"The critical shortage here is not stuff, but time." - Johan Ekdahl

 

"Step N is required before you can do step N+1!" - ka7ehk

 

"If you want a career with a known path - become an undertaker. Dead people don't sue!" - Kartman

"Why is there a "Highway to Hell" and only a "Stairway to Heaven"? A prediction of the expected traffic load?"  - Lee "theusch"

 

Speak sweetly. It makes your words easier to digest when at a later date you have to eat them ;-)  - Source Unknown

Please Read: Code-of-Conduct

Atmel Studio6.2/AS7, DipTrace, Quartus, MPLAB, RSLogix user

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

jgmdesign wrote:

Just because a device is in the AVR family does not mean that you can write code(or use code pre-written) for one device and expect it to work on a completely different device.

 

You will need to adjust registers accordingly.

 

What is the desired function of your widget?  Does it need USB?  If it does then you have to set up a USB driver for the AVR >>LUFA for example<<  If you do not need usb then the Mega328 should be able to do what you need. 

 

If you are a beginner with all of this and you need USB, I would strongly suggest you stick with the Mega328 and use an external USB part.  Like the ones from FTDI, Silicon Labs, or the CH340.

 

 

Write down on paper what you want to achieve, and post it here.  Then lets set this up properly.

JIm

Yes I am aware of this, as stated above, I have adapted the registers for the new Pinout.

So today I compiled a USB Serial example from the LUFA library and used it to debug on the board and finally got it to work. I am half certain I must have had a wrong pin number somewhere in the version from yesterday, because I finally got it to work with very few alterations to the sketch.

For anybody interested I'll attach the working solution. It assumes the 4x4 pad is attached to digital pins 2-9 on an ATmega32u4, so I only hardcoded the pinout for those 9 pins. It's probably a good idea to init arrays with all the pin ports then define a getbyindex to that instead of actually calling functions with if cases, so less overhead is generated. The getKeyDown method is also really not elegant but it all works quite nicely for now. Refactoring can wait for after Christmas ;)

I appreciate all the help here, thanks everyone and have a good holiday season!

Attachment(s):