KeyMatrix

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

Hi everyone.
I have a fifty button matrix here and I have written some code to poll it.
I am using an led to show that when a button is pressed the code is actually working.
The first thing is that the led is dim when I press one button, but as I press more it gets brighter.
The second is I would like to understand how to make it more efficient - the code I mean. I understand that you can read an entire ports state at once, and so I think doing something like this would be quicker than one I have done to poll the state of all the buttons?
Please can someone show some improvements I could make:

void KeyMatrix::pollKeys(){

	for (int i = 0; i < 8; i++) {
		DDRK |= (1<<i); //output
		PORTK &= ~(1 << i); //low
		for (int j = 0; j < 8; j++) {
			if (!(PINH & (1 << j))) {
				output_low(PORTB, ENCLEDRED);
			} else
			output_high(PORTB, ENCLEDRED);
		}
		PORTK |= (1 << i);
		DDRK &= ~(1 << i);
	}
}

P.S Using mega2560 here.

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

Since it is working.. No issues.. I would like to know in which part of the code, you set PORTH as input and enable its pullup...

Regarding a poor glowing.. How is the LED connected??

Is the micro controller sourcing or sinking?

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

You have not initialised PORTK or PORTH or even PORTB.

I presume that you have an 8x8 matrix, and just connect 50 of the possible 64 nodes.

Normally you will have pull-ups on all the rows.
Then ground one column at a time.
Ideally you have the other columns as pulled-up inputs.

Reading each row will show a 0 if a key is pressed.

void KeyMatrix::pollKeys(){

   for (int i = 0; i < 8; i++) {
      DDRK = (1<<i); //output one col with all others input
      PORTK = ~(1<<i); //low one col with all others pull-up
      for (int j = 0; j < 8; j++) {
         if (!(PINH & (1<<j))) {
            output_low(PORTB, ENCLEDRED);
         } else
         output_high(PORTB, ENCLEDRED);
      }
   }
}

Untested. Note that using a mask is more efficient than (1<<j)

I can't see any reason for your LED to glow dim/bright. OTOH, we have no idea of your wiring or SFR initialisation.

However your LED is going to be changed on every 64 iterations. You normally want to show a human something visible.

David.

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

Sorry I just displayed the actual poll algorithm. Here is my init()

	set_output(DDRB, ENCLEDRED);
	output_high(PORTB, ENCLEDRED);

	//Prepare the rows for reading
	DDRH = 0b00000000; //All Ts are inputs
	PORTH = 0b11111111; //All pullups enabled
	//read from PINH

	DDRK = 0b11111111;//MK and BR are outputs - one is pulled low at a given time.
	PORTK = 0b11111111;	//Set them high for now. We can pull one low when scanning.

Is the only difference removing the

    PORTK |= (1 << i); 
      DDRK &= ~(1 << i);

?
I'll try a different method of feedback like serial. See what gets displayed

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

No, read the message.

If you have all the columns as outputs, what happens when you press two adjacent keys on the same row ?

David.

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

Oh yeah, hadnt spotted that. (the = with no preface).

How would you do it with a mask? I thought bit shifting was just as fast (cant remeber where I read that)

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

a.mlw.walker wrote:
How would you do it with a mask? I thought bit shifting was just as fast (cant remeber where I read that)
Not bit shifting in general is the problem, but shifting by more than one position, or even worse by a variable number of positions.

   for (int i = 0; i < 8; i++) {
      DDRK = (1<<i);

->

    for (uint8_t mask = 0x01; mask; mask <<= 1) {
        DDRK = mask;

Stefan Ernst

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
   for (uint8_t mask = 1; mask != 0; mask <<= 1) {

this is the same:

   for (uint8_t mask = 1; mask != 0; mask += mask) {

Most likely the <<= will just be a single ADD reg, reg statement.
(1 << variable) will be multiple statements in a loop.

Of course, some MCUs have a barrel-shifter. The AVR does not.

Note that worrying about what a C compiler might generate for two equally valid C sequences is absolutely stupid.

David.

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

Can I do that for all of the bit shift I currently have i.e:

void KeyMatrix::pollKeys(){
	for (uint8_t mask = 1; mask != 0; mask += mask) { 
		DDRK = mask;
      PORTK = ~mask; //low one col with all others pull-up
      for (uint8_t mask2 = 1; mask2 != 0; mask2 += mask2) { 
         if (!(PINH & mask)) {
        	 serial.putS("pin high");
            output_low(PORTB, ENCLEDRED);
         } else
        	 serial.putS("pin low");
         output_high(PORTB, ENCLEDRED);
      }
   }
}

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

Yes.

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

The only thing then that still concerns me is I have a loop within a loop. Surely as the loops are 8 and a byte stores 8 bits it must be possible to do this with 1 or even no loops?

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

You can skip the inner loop if you test for 0xFF first.

Remove your 'complex' serial calls.

Then simulate the nested loop. I doubt if the total cycles comes to much more than 1000. i.e. 63us @ 16MHz.

You probably only do a keyscan once every 20000us.

David.

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

As in test for 0xFF like this:

if (!(PINH & 0xFF)) { 
output_low(PORTB, ENCLEDRED); 
         } else 
            output_high(PORTB, ENCLEDRED); 

Or do I still need to test for:

if (!(PINH & mask)) { 

aswell?

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

Alex,

Just put your brain into gear. If no keys are pressed in that column, the row will read as 0xFF.

There is no point in looking for which bit(s) might be 0.

How do you test for equality with a value?

David.

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

Of course!

for (uint8_t mask = 0x01; mask; mask <<= 1) {
		        DDRK = mask;
		      PORTK = ~mask; //low one col with all others pull-up
		         if ((PINH != 0xFF)) {
		            output_low(PORTB, ENCLEDRED);
		         } else
		         output_high(PORTB, ENCLEDRED);
		   }

And then if it doesnt equal 0xFF I can then check each bit if I want to know which one it is.
If however I want to know which button in the matrix it is, and therefore PINH != 0xFF then is it best to then run another mask at that point to find out which one it is? What would be a good way to ID the button using this method? Have an array that stores the state of every button and put it in there? How do I map to it? I.e if I want to output a 28 when the 28th button is pressed - which would be the fourth row first column.