Combined LED and switch matrix...

Level: 10k+ Postman

Joined: Thu. Dec 30, 2004

Posts: 23019 View posts

Location: Melbourne,Australia

#2

Posted by Kartman: Wed. Sep 6, 2017 - 11:02 PM

After the resistors might afford a degree of ESD protection. You would not want led current flowing when reading the switches though.

awneil

Level: 10k+ Postman

Joined: Fri. Jul 1, 2005

Posts: 17121 View posts

Location: Basingstoke, Hampshire, UK

#3

Posted by awneil: Thu. Sep 7, 2017 - 06:50 AM

Why would you want to go from a (presumably working) TQFP design to DIP ??

Especially when that specifically creates these problems for you !

Top Tips:

How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
"Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
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
Difference between a crystal, and a crystal oscillator: https://www.avrfreaks.net/comment...
When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#4

Posted by joeymorin: Thu. Sep 7, 2017 - 11:52 AM

I assume you've looked at AVR242?

8-bit Microcontroller Multiplexing LED Drive and a 4 x 4 Keypad

Although it's for 4x7-segment and 4x4 keypad, the principles are the same.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#5

Posted by alank2: Thu. Sep 7, 2017 - 12:10 PM (Reply to #3)

awneil wrote:

Why would you want to go from a (presumably working) TQFP design to DIP ??

Especially when that specifically creates these problems for you !

To make a kit that others can build without SMT skills.

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#6

Posted by alank2: Thu. Sep 7, 2017 - 12:13 PM

joeymorin wrote:

I assume you've looked at AVR242?

8-bit Microcontroller Multiplexing LED Drive and a 4 x 4 Keypad

Although it's for 4x7-segment and 4x4 keypad, the principles are the same.

I may have years ago. It is interesting that they are using all the column pins for both columns and rows in the keypad.

Paulvdh

Level: Raving Lunatic

Joined: Fri. Aug 3, 2001

Posts: 2802 View posts

#7

Posted by Paulvdh: Thu. Sep 7, 2017 - 12:59 PM

HT16k33 does all the multiplexing and keyboard scanning for you for USD 50 cents. from Ali / Ebay.

for USD 1 they come soldered on a breakoutboard with pins.

But for multiplexing led's and keyboard you need to pay some attention to not short out the leds.

*ANY* uC manufacturer probably has an AN about that. AVR242 is a good example, but the firmware example is a bit oldfashioned ASM.

So it's doable, pretty straightforward, and as always, propper debouncing is probably the hardest to get right.

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

skeeve

Level: Raving Lunatic

Joined: Sun. Oct 29, 2006

Posts: 5098 View posts

#8

Posted by skeeve: Thu. Sep 7, 2017 - 05:37 PM

Charlieplexing the LEDs would cost you 6 pins.

Similar with the switches would cost you 7 pins and a diode per switch.

If only one switch at a time might be closed,

you can get rid of the additional diodes by sharing the LED pins

and using a straightforward row-column mechanism on the switches.

Iluvatar is the better part of Valar.

theusch

Level: 10k+ Postman

Joined: Mon. Feb 19, 2001

Posts: 39612 View posts

Location: Wisconsin USA

#9

Posted by theusch: Thu. Sep 7, 2017 - 06:41 PM

Indeed I have my ideas on the utility of this snipe hunt.

Another way to approach is with '165 and '895 or equivalent as "port expanders". '895 has pretty good drive for LED work.

That said, I've got an app with 6x 7-segments, and three inputs for each of the six stations. Two AVR ports. Lessee, that would be 48 "LEDs" and 18 "switches".

// PORTS (Mega164 Function)
//-----------------------------------------------------------------------------
//	PIN	Mega164 USE		DIR	INIT	DESCRIPTION
//	===	===========		===	====	===========
//
// PA.7 (ADC7)			In	(z)		unused in this application; tied low
// PA.6 (ADC6)			In	(z)		Raw VUN nominal +24VDC monitor, 10k/2k ==> 1/6
// PA.5 (ADC5)			Out	(0)		7-segment LED6 (WELL6) column strobe, SW1/SW2/DIS1 strobe
// PA.4 (ADC4)			Out	(0)		7-segment LED5 (WELL5) column strobe, SW3/SW4/DIS2 strobe
// PA.3 (ADC3)			Out	(0)		7-segment LED4 (WELL4) column strobe, SW5/SW6/DIS3 strobe
// PA.2 (ADC2)			Out	(0)		7-segment LED3 (WELL3) column strobe, SW7/SW8/DIS4 strobe
// PA.1 (ADC1)			Out	(0)		7-segment LED2 (WELL2) column strobe, SW9/SW10/DIS5 strobe
// PA.0 (ADC0)			Out	(0)		7-segment LED1 (left; WELL1) column strobe, SW11/SW12/DIS6 strobe

// PB.7 (SCK)			Out	(1)		7-segment DP
// PB.6 (MISO)			Out	(1)		7-segment G
// PB.5 (MOSI)			Out	(1)		7-segment F
// PB.4 (!SS/OC0B)		Out	(1)		7-segment E
// PB.3 (AIN1/OC0A)		Out	(1)		7-segment D
// PB.2 (AIN0/INT2)		Out	(1)		7-segment C
// PB.1 (CLKOT1)		Out	(1)		7-segment B
// PB.0 (XCK0/T0)		Out	(1)		7-segment A

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.

Paulvdh

Level: Raving Lunatic

Joined: Fri. Aug 3, 2001

Posts: 2802 View posts

#10

Posted by Paulvdh: Thu. Sep 7, 2017 - 07:22 PM

Charlieplexing is ... quite a mess in my opinion.

quite a headacke to build & program, but it can (and has) been done (multiple times over).

An alternative to multiplexing the keys is to put a string of resistors between them and measure them with a single ADC input.

Doing magic with a USD 7 Logic Analyser: https://www.avrfreaks.net/comment/2421756#comment-2421756

Bunch of old projects with AVR's: http://www.hoevendesign.com

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#11

Posted by alank2: Thu. Sep 7, 2017 - 07:51 PM

I too find Charlieplexing not worth the trouble when multiplexing works so darn well.

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#12

Posted by joeymorin: Thu. Sep 7, 2017 - 11:10 PM

YMMV of course, but I find Charlieplexing no more difficult than 'normal' multiplexing, in design, layout, or software. It is slightly more cpu intensive, but the difference is scarcely measurable. Even if it were significantly more difficult to code, once you've written it once, it is easily reusable. I can hardly think of a reason >>not<< to use it when pins are scarce. Duty cycle is comparable. The only reason in practice to skip it is if higher drive current is required than can be sinked by a single I/O pin and shared over N LEDs. Who would want to drive 7 7segment displays with more than 8 pins?

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#13

Posted by alank2: Fri. Sep 8, 2017 - 12:38 AM

I would love to see an example of charlieplexing code that is optimized if you have any you can post joeymorin. I bought a couple of charlieplex LED boards from adafruit or something and would like to see code that could drive them cleanly. This is why I've always liked multiplexing. Disable a row, change all the columns, enable the row, wait to do it again. The last time I looked at charlieplexing I would thinking of how I could store the actions it would need to go through in an array to be traversed, but maybe I was overthinking it.

El Tangas

Level: Raving Lunatic

Joined: Thu. Jul 14, 2016

Posts: 2762 View posts

#14

Posted by El Tangas: Fri. Sep 8, 2017 - 01:50 AM

My suggestion is similar to #7, but I'll mention different chips: Titan Micro TM1629 or TM1629D, they can control lots of LEDs and 32 keys. The price is about the same as Holtek HT16k33.

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#15

Posted by joeymorin: Fri. Sep 8, 2017 - 05:52 AM (Reply to #14)

The last time I looked at charlieplexing I would thinking of how I could store the actions it would need to go through in an array to be traversed, but maybe I was overthinking it.

Simple masks arranged in an array are sufficient to maintain state.

With an N-pin Charlieplex matrix, you have:

N²-N nodes (LEDs)
Duty cycle of 1/N
N steps/phases per scan cycle
An array of N masks which govern the state of the array

I would love to see an example of charlieplexing code that is optimized if you have any you can post

I use a handful of small LUTs to:

map application node order onto raw node order
map a raw node to its anode number
map a raw node to its cathode mask

This greatly simplifies the implementation and, I think, the readability of the code. The ISR which runs the scan engine is effectively 4 lines of code: blanking, new mask, drive, next phase. The function which modifies the state of the array is also effectively 4 lines of code: 3 lookups, and a write.

Below is a complete example of an N=5 Charlieplex matrix on an ATtiny25/45/85 (should work on a t13), although it could easily be expanded to N=8 on any AVR with an 8-bit port. N>8 is only a matter of increasing the width of the mask and mapping the appropriate parts to more than one port, or using a two dimensional array and doing the same. Left as an exercise for the reader.

It's basically the same initial demo code I wrote when I first played with Charlieplexing on a one-off demo board I built around a t85, 20 high-efficiency red LEDs, and 3 LR44 button cells. The button cells' high ESR meant that I could omit the cathode resistors altogether without fear of letting any smoke out. The lack of current limiting and low impedance PS does mean that node brightness varies with the number of driven nodes per anode/phase, but it's only a quick and dirty demo so I didn't mind. The only passive is a 100 nF bypass.

I gussied up the code a bit over the original, including lengthier comments to post here, and updated it a bit to use __flash instead of PROGMEM. If you don't have a t85, it's easily ported to any other AVR. Key things to change would be the init function, and the vector name. The timer configuration choices are made at compile time with macros and conditionals, so if you're using a timer with different resolution or different precaler taps, you'll want to tickle those as well.

The LUT which maps application node order onto raw node order lets me assign numbering scheme I wish to the array. The mapping below results in a raster scan of the 4x5 arrangement of the 20 LEDs on the demo board, as shown in the attached video.

I used only 2 LR44 for ~3V for the video, because the LEDs overwhelmed my camera's sensor at 4.5V. Clock speed was 1 MHz, refresh rate 125 Hz, for an ISR rate of 625 Hz. That's 1600 cycles per ISR pass. The ISR has a w.c.e. of 72 cycles including the response time and vector. That's 4.5% cpu utilisation to run the engine.

EDIT: A bit more gussied.

Attachment(s):

charlieplex_demo.avi_.zip

charlieplex_demo.c.zip

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

Last Edited: Sun. Sep 10, 2017 - 05:21 PM

MicroGyro

Level: Hangaround

Joined: Tue. Jul 12, 2011

Posts: 361 View posts

#16

Posted by MicroGyro: Fri. Sep 8, 2017 - 06:42 AM

Please try this:

ATmega32 at 8 MHz

#define SCANNER_DDR   DDRC
#define SCANNER_PORT  PORTC
#define SCANNER_PIN   PINC

unsigned char scanner_bit;


ISR(TIMER0_COMP_vect)
{
/***>>> 4.992 ms period is for LEDS and KEYs scans <<<***/
 
 SCANNER_DDR  = 0x00;
 SCANNER_PORT = 0xFF;
 SCANNER_DDR  |= (1<<scanner_bit);
 SCANNER_PORT &= ~(1<<scanner_bit);
 if(++scanner_bit==4) scanner_bit=0;

/* KEYs and LEDs scan */

 if(!(SCANNER_PIN & (1<<6)) && (scanner_bit==1)) //button at row 1 col 3
 {
  SCANNER_DDR = 0x41; //set output for LED at row 1 col 3
  SCANNER_PORT= 0x01; //turn on LED
 }

 if(!(SCANNER_PIN & (1<<7)) && (scanner_bit==2)) //button at row 2 col 4
 {
  SCANNER_DDR = 0x82; //set output for LED at row 2 col 4
  SCANNER_PORT= 0x02; //turn on LED
 }

}


void init(void)
{
// Timer/Counter 0 initialization
// Clock source: System Clock
// Clock value: 7.813 kHz
// Mode: CTC top=OCR0
// OC0 output: Disconnected
// Timer Period: 4.992 ms
TCCR0=(0<<WGM00) | (0<<COM01) | (0<<COM00) | (1<<WGM01) | (1<<CS02) | (0<<CS01) | (1<<CS00);
TCNT0=0x00;
OCR0=0x26;
// Timer(s)/Counter(s) Interrupt(s) initialization
TIMSK=(0<<OCIE2) | (0<<TOIE2) | (0<<TICIE1) | (0<<OCIE1A) | (0<<OCIE1B) | (0<<TOIE1) | (1<<OCIE0) | (0<<TOIE0);
}


int main(void)
{
 init();
 sei();

 while(1) 
 {

 }
}

Forgive me for magic numbers. I'm too lazy to fix it.

Tested in Sim (no complain) and Real (only 2 button tested randomly). Multi button press not supported. Find a way to it.

Hope it can give you better idea.

MG

I don't know why I'm still doing this hobby

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#17

Posted by alank2: Sat. Sep 9, 2017 - 01:43 PM

MG interesting combining the rows using diodes, very cool.

I got it breadboarded last night - what is everyone's take on switching the rows between output low and HiZ vs. switching the rows between output low and output high.

I started with using HiZ/output low and to get a clean reading from the keyboard (which I know I have to debounce properly, that won't be an issue), I have to use more NOP's than I think I should. It seems like 4 NOP's should be the correct amount to let the input pin synchronizing get updated.

It is real touchy - depending on the order of the commands.

        //turn off LED row
        PORTB=0;           //clear before switching to input
        DDRB=0;            //switch columns to input
        DDRA&=~_BV(row+4); //LED row from output low to HiZ
        
        //advance row
        row++;
        if (row==4)
          row=0;


        //scan keyboard
        PORTB=0xff;        //enable pullups
        DDRA|=_BV(row);    //KB row from HiZ to output low

        asm("nop");        //wait for stable input
        asm("nop");
        asm("nop");
        asm("nop");
        asm("nop");        //wait for stable input
        asm("nop");
        asm("nop");
        asm("nop");
        
        leds[row]=PINB;    //read input
        DDRA&=~_BV(row);    //KB row from output low to HiZ


        //turn on LED row
        PORTB=leds[row];   //set columns to what we want to display
        DDRB=0xff;        //columns output
        DDRA|=_BV(row+4);  //LED row from HiZ to output low

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#18

Posted by joeymorin: Sat. Sep 9, 2017 - 03:23 PM

I have to use more NOP's than I think I should. It seems like 4 NOP's should be the correct amount to let the input pin synchronizing get updated.

With pullups, you have to consider the TC of the filter created by the pull-up resistor and the combined capacitance of anything which might be connected to the pin. You have to do the math.

The pull-up has an upper bound of 50K. A single pin's capacitance is in the neighbourhood of 10 pF. If that is all you needed to worry about, then the TC would be 500 us ns.

If you're running at 8 MHz, then yes, that's 4 cycles. One TC is sufficient to swing an input from 0V to 0.632*V_CC, which should be enough to read as a '1'. However you've probably got a more than 10 pF of capacitance. Possibly a lot more. Let's say, 400 pF. Could be significantly higher if you've got long ribbon cables going to the keyboard. 50K*400pF = 20 us. At 8 MHz, that's 160 cycles. At 1 MHz, its 20 cycles.

If the pullup is closer to 20K instead of 50K, The TC with 400 pF is 8 us, or 8 cycles at 1 MHz, but you should design for worst case.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

Last Edited: Sun. Sep 10, 2017 - 03:39 AM

MicroGyro

Level: Hangaround

Joined: Tue. Jul 12, 2011

Posts: 361 View posts

#19

Posted by MicroGyro: Sat. Sep 9, 2017 - 03:32 PM

Well that's a better idea reading button using array and later use it as led bit. Good idea.
Can't you replace the nop with other 8 cycle task? I rarely use nop nor delay in my code unless no other way. But it's up to you.

alank2 wrote:

switching the rows between output low and output high.

On my schematics if row is out-hi then any leds connected to same column will lit if a button at that column get pressed. That's why I use hi-z.
.
MG

I don't know why I'm still doing this hobby

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#20

Posted by alank2: Sat. Sep 9, 2017 - 04:33 PM (Reply to #19)

MicroGyro wrote:

Can't you replace the nop with other 8 cycle task? I rarely use nop nor delay in my code unless no other way. But it's up to you.

Yes, there are plenty of other things I need to do - you guys are always thinking brilliantly!!!

theusch

Level: 10k+ Postman

Joined: Mon. Feb 19, 2001

Posts: 39612 View posts

Location: Wisconsin USA

#21

Posted by theusch: Sun. Sep 10, 2017 - 12:32 PM

alank2 wrote:

I have to use more NOP's than I think I should.

??? I'm losing track a bit, but why race? Read the inputs per previous setup. Apply new setup for next multiplex period. Let multiplex period elapse. Rinse and repeat; no NOP in sight.

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.

JohanEkdahl

Level: 10k+ Postman

Joined: Wed. Mar 27, 2002

Posts: 26988 View posts

Location: Lund, Sweden

#22

Posted by JohanEkdahl: Sun. Sep 10, 2017 - 02:07 PM

Side note: In joeymorins attached code:

#define ISAPOWEROFTWO(x) ((x) && (!((x) & ((x) - 1))))

Cunningly Clever Canadian.. ;-)

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here.

No guarantees, but if we don't report problems they won't get much of a chance to be fixed! Details/discussions at link given just above.

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

DocJC

Level: 10k+ Postman

Joined: Tue. Dec 11, 2007

Posts: 12738 View posts

Location: Cleveland, OH

#23

Posted by DocJC: Sun. Sep 10, 2017 - 05:37 PM

Engineering is all about trade offs...

How will the device be powered, and does a little bit more current consumption matter?

Why not use MicroGyro's schematic, above, but use four EXTERNAL pull-up resistors?

Use four 1K, (or whatever you elect), external pull up resistors on the micro's pins that go to the diodes.

You now don't have to worry about a 50K pull-up and perhaps some significant line capacitance, as you have made the R much, much lower, and hence the Time Constant is much, much lower.

Now 1 NOP might be sufficient?

The external pull-up won't impact driving the multi-plexed LEDs, except for the fact that when a column is pulled low there will be some extra current flowing through the pull-up as the pin pulls it low.

The code, in this case, doesn't have to switch the internal pull-up resistors on and off.

This adds four resistors to your project's kit, perhaps not a bad thing if this is for beginners, soldering practice, etc.?

Nobody every used the terms professional coder and my name in the same sentence, but if I were to code the driver for this I might think about coding the multiplexed LED matrix on a Timer/Counter CTC mode ISR, or whatever reasonably fast "Tic" the system has.

As I started execution of each LED ISR, before I configured the next LED settings, I would take a quick read of one of the switch matrix rows or columns.

That would give equal spacing to the LED matrix illumination process, and would only take a couple of clock cycles immediately before each LED column is turned on.

If you additionally set a flag with each 4th time through the LED Matrix ISR, and hence each 4th time through the switch matrix, your Main Loop and / or debounce routine can process the switch matrix data only every 4th time, i.e. full scan, of the switch matrix, (and reset the flag...).

The trade off: Make the time constant essentially a non-issue, and speed up the LED & Switch Matrix driver code, at the expense of four pull-up resistors that will draw current when their section of the grid is being driven.

JC

Edit: Typo

Last Edited: Sun. Sep 10, 2017 - 05:39 PM

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#24

Posted by joeymorin: Sun. Sep 10, 2017 - 10:00 PM

Cunningly Clever Canadian

Yes, I Googled it with my bare hands ;-)

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

MicroGyro

Level: Hangaround

Joined: Tue. Jul 12, 2011

Posts: 361 View posts

#25

Posted by MicroGyro: Mon. Sep 11, 2017 - 01:18 AM

DocJC wrote:

Use four 1K, (or whatever you elect), external pull up resistors on the micro's pins that go to the diodes.

My old brain processor failed to process it. Can you give a simple drawing? I want to try it. Good idea always welcome for me.
.

DocJC wrote:

multiplexed LED matrix on a Timer/Counter CTC mode ISR, or whatever reasonably fast "Tic" the system has.

Why need so fast? Isn't debounce and display don't need that fast? Forgive my misunderstanding.
.
MG

I don't know why I'm still doing this hobby

DocJC

Level: 10k+ Postman

Joined: Tue. Dec 11, 2007

Posts: 12738 View posts

Location: Cleveland, OH

#26

Posted by DocJC: Mon. Sep 11, 2017 - 12:43 PM

The 1 K, (or whatever) resistors replace the internal pull-up resistors.

The scan rate has a lot of latitude.

With four rows, each row will be on for slightly less then 1/4 of the time.

If one wants to prevent any visible flicker from slow scanning, then one has to select a fast enough scan rate.

Scanning the entire array of four rows at 100 Hz is certainly reasonable.

That means updating each row at roughly 400 Hz scan rate is reasonable.

That means an ISR for the LED driver fires at once every 2.5 mSec.

I would tightly couple the push button matrix reading with the LED scanning, so as to maximize the On time for each row of LEDs, to maximize their brightness.

At really slow scan rates the LEDs flicker, but the overhead, processor wise, for the scanning code is negligible.

At really fast scan rates the LEDs won't flicker, but the overhead for the scanning code, (whether done via ISR or not), becomes significant.

(And additionally, the 1/4 % duty cycle per LED starts to get a bit shorter with very fast scan rates...)

Yet another Engineering Decision / trade off.

Reading push button switch data once per every 1 or 2 mSec is certainly reasonable, although one could slow that process down if desired.

The extra overhead to read the push button switches every 4th pass of the LED scanner just doesn't seem worth the effort to me.

One just sets the debounce counter a little higher with the faster test rate.

JC

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#27

Posted by alank2: Mon. Sep 11, 2017 - 01:33 PM

My breadboard is coming together pretty nicely now. I ended up using 5 columns and 6 rows each for the LED and KB so the rows are separate. I had an issue where some defective LED leaking caused switch problems and I have enough pins I don't need to share the rows and use diodes. I needed 31 buttons, but as it turned out, one of them needs to be connected to INT0 for a deep sleep wakeup, so 5x6=30 for the remaining buttons. It is connected up with a ton of wires going every which way but it works very well.

MicroGyro

Level: Hangaround

Joined: Tue. Jul 12, 2011

Posts: 361 View posts

#28

Posted by MicroGyro: Mon. Sep 11, 2017 - 11:59 PM

DocJC wrote:

The 1 K, (or whatever) resistors replace the internal pull-up resistors.

Got it! Thanks Doc.

alank2 wrote:

It is connected up with a ton of wires going every which way but it works very well.

Glad you succeeded.
.
MG

I don't know why I'm still doing this hobby

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#29

Posted by joeymorin: Tue. Sep 12, 2017 - 12:36 AM

For later...

I bought a couple of charlieplex LED boards from adafruit or something and would like to see code that could drive them cleanly.

I assume you mean one of these:

https://www.digikey.com/product-detail/en/adafruit-industries-llc/2948/1528-1630-ND/5959342

... or another colour.

Note that these boards were made to be driven by this:

https://www.digikey.com/en/product-highlight/a/adafruit/is31fl3731-matrix-led-driver-breakout-board

... which is an I2C device, no need to drive the LEDs directly with a software Charlieplexing solution.

They can, of course, be driven however you like. Note, however, that this board is a 16x9 array for a total of 144 LEDs, but is in fact two side-by-side 8x9=72-LED arrangements of N=9 Charlieplex arrays. As such, each N=9 array requires 9 pins to drive. The demo code I posted in #15 is made for up to N=8. As mentioned, it is straightforward to adapt to N>8 by using more than one I/O port. If you get to it some day and need a hand, just let me know.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

MicroGyro

Level: Hangaround

Joined: Tue. Jul 12, 2011

Posts: 361 View posts

#30

Posted by MicroGyro: Tue. Sep 12, 2017 - 03:31 AM

@joeymorin
I didn't see any schematic of your demo project so it's hard to know the leds wiring from what your code doing.
Or maybe I missed it?
.
MG

I don't know why I'm still doing this hobby

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#31

Posted by joeymorin: Tue. Sep 12, 2017 - 03:52 AM

I didn't post one. It's just a standard Charlieplex matrix. Nothing fancy.

Here's an N=3 matrix, with 3 pins, and N²-N = 6 nodes.

The code in #15 is for N=5, but it's easily adapted to any N<=8. N>8 is a smidgen harder, as it requires more than one port, but not at all difficult. The schematic for any N is just an extension of the above. Basically, there are two LEDS between each pair of pins, one reverse biased w.r.t. to it's partner. There are only N resistors, one for each I/O pin, placed between the I/O pin and the cathodes of all of the LEDs which share that I/O pin. All LED anodes are connected directly to their respective I/O pins, with no resistor. Easy peasy lemon squeezy.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

MicroGyro

Level: Hangaround

Joined: Tue. Jul 12, 2011

Posts: 361 View posts

#32

Posted by MicroGyro: Tue. Sep 12, 2017 - 04:07 AM

joeymorin wrote:

Easy peasy lemon squeezy.

For expertise like you. For me maybe needs a week to figure it out :)
Thanks anyway.
.
MG

I don't know why I'm still doing this hobby

El Tangas

Level: Raving Lunatic

Joined: Thu. Jul 14, 2016

Posts: 2762 View posts

#33

Posted by El Tangas: Tue. Sep 12, 2017 - 11:17 AM

The datasheet of the IS31FL3731 is very useful because it shows a nice way to arrange the LEDs for Charlieplexing. The resistors could also be added more or less easily.

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#34

Posted by joeymorin: Tue. Sep 12, 2017 - 12:24 PM

The datasheet of the IS31FL3731 is very useful because it shows a nice way to arrange the LEDs for Charlieplexing.

If you want them arranged in an matrix, yes. However, that's not the only way a Charlieplexing can be used:

https://www.google.ca/search?q=charlieplex+clock

One of the projects I had in mind for Charlieplexing is a version of this:

https://en.wikipedia.org/wiki/Kenbak-1

... using an ATtiny85. 12 LEDs, 15 (or 16) switches, total 27 (or 28) nodes. So, N=6 should be enough, with diodes in series with each switch node. The t85 has only 5 'normal' I/O pins, but /RESET can be used of course (and I have an HVSP programmer), but it has reduced driver strength. Not a problem if it is used solely for switch nodes.

Alternately, I could use a resistor divider arrangement for the switches, connected to 3 ADC inputs (one of which would be /RESET), 5 switches per ADC input, and an N=3 arrangement for the LEDs on the remaining pins.

Someone already made one with a 328P:

https://hackaday.com/2011/09/30/recreating-the-first-pc/

For expertise like you.

Ha!

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

Last Edited: Tue. Sep 12, 2017 - 12:34 PM

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#35

Posted by joeymorin: Tue. Sep 12, 2017 - 10:11 PM

CONTINUED_CHARLIEPLEX_OT:

I didn't see any schematic of your demo project so it's hard to know the leds wiring from what your code doing.

Here are a couple of references:

https://www.avrfreaks.net/forum/controlling-many-output-lines-just-few-gpio-lines

https://www.avrfreaks.net/forum/charlieplexing-leds-microcontroller-refresh-rate

The latter has an animation which may help.

/CONTINUED_CHARLIEPLEX_OT

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

MicroGyro

Level: Hangaround

Joined: Tue. Jul 12, 2011

Posts: 361 View posts

#36

Posted by MicroGyro: Wed. Sep 13, 2017 - 12:13 AM

@joeymorin
Thank you for the links, I'll check it in my spare time. :)
.
MG

I don't know why I'm still doing this hobby

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#37

Posted by alank2: Wed. Sep 13, 2017 - 12:41 PM

So guys, I got my project mostly working last night, but the time it takes to get accurate keyboard results is still quite a bit higher than I want. You'll think it is crazy, but I am running a 5kHz ISR on an 8MHz clock which I am sure is consuming a lot of cpu bandwidth, but the main loop code doesn't have to run fast and it is plenty responsive. I've got 6 rows and the reason I am running the ISR so fast is to get 8 levels of brightness. 5000/6/8 = 104 Hz. Most of the executions are merely do we need to turn off the LED row yet? Every 8 of them however it switches rows leaving me with a 625 Hz speed for scanning the keyboard. More often I run an ISR at 1K and sample the keyboard at that speed and then debounce it. What I setup last night was scanning the entire keyboard (all 6 rows) at 625 Hz, but the time between switching them is 32 cycles to get a working reliable keyboard for all keys. I can't come up with something to do for each of these 6 row waiting periods, so I am just doing delays for now.

One solution would be to move to scanning only one row at a time, but this would reduce my keyboard scanning frequency to 104 Hz or just under 10ms between scans of the entire keyboard. I am debouncing right now by having 30 milliseconds of consistency before it accepts the new state. I could move to the 10ms and have 3 10ms groups of consistency and see how that works.

Alternatively, and perhaps additionally to that solution is are there methods I can use to reduce the time it takes. My testing originally showed that the order of the commands would make a difference. In this case, I am going from LED driving mode (columns output, maybe on maybe off) to KB scanning mode (columns input with pullup enabled). At this point in the code, all the LED rows are in HiZ. Right now, I am using 470 ohm resistors in line with the columns going to both the keyboard and LED matrix. I had thought about using the resistors only to go to the LED matrix and leaving the keyboard off, but I somewhat liked the safety of having a resistor on those pins in case something were to go wrong in code and leaving columns in a high state and keyboard rows in a low state with no resistance between them.

Also, I found that while I am using independent rows for the LED matrix and the keyboard, that pressing two LED's in a column will allow current to leak out of one column and leak into another when it is displaying the LED matrix. I am guessing I could use 5 column diodes only to prevent that from happening.

Then there is I could set the active column I want to scan to output low and use the row side with pullups to scan for pressed buttons - would that be any faster potentially?

        LBKBSHARED_PORT|=_BV(LBKBSHARED_COL0_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL4_PIN);
        DDR(LBKBSHARED_PORT)&=~(_BV(LBKBSHARED_COL0_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL4_PIN));

        //76543210 76543210 76543210 76543210
        // 0000011 11122222 33333444 4455555s

        #define SETTLE_DELAY 32

        //row 0
        DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW0_PIN);
        __builtin_avr_delay_cycles(SETTLE_DELAY);
        ((uint8_t*)&AInput)[0]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) >> 1;
        DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW0_PIN);

        //row 1
        DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW1_PIN);
        __builtin_avr_delay_cycles(SETTLE_DELAY);
        ((uint8_t*)&AInput)[0]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN))) >> 6;
        ((uint8_t*)&AInput)[1]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) << 2;
        DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW1_PIN);

        //row 2
        DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW2_PIN);
        __builtin_avr_delay_cycles(SETTLE_DELAY);
        ((uint8_t*)&AInput)[1]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) >> 3;
        DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW2_PIN);

        //row 3
        DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW3_PIN);
        __builtin_avr_delay_cycles(SETTLE_DELAY);
        ((uint8_t*)&AInput)[2]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN)));
        DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW3_PIN);

        //row 4
        DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW4_PIN);
        __builtin_avr_delay_cycles(SETTLE_DELAY);
        ((uint8_t*)&AInput)[2]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN))) >> 5;
        ((uint8_t*)&AInput)[3]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) << 3;
        DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW4_PIN);

        //row 5
        DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW5_PIN);
        __builtin_avr_delay_cycles(SETTLE_DELAY);
        ((uint8_t*)&AInput)[3]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) >> 2;
        DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW5_PIN);

        //columns output
        DDR(LBKBSHARED_PORT)|=_BV(LBKBSHARED_COL0_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL4_PIN);

Last Edited: Wed. Sep 13, 2017 - 12:42 PM

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#38

Posted by alank2: Thu. Sep 14, 2017 - 02:47 PM

Last night I moved to scanning a single row for each row of the LED matrix being run. The change from (30) 1.6ms debounce periods to (3) 9.6 seemed to make a difference in the delays I need to use as well. 2 cycle delay would not work, 3 cycle would work on most keys, 4 cycle worked on all keys, so I am using 8 cycle just to make sure. The ISR is still less than 6% of the total uC cycle usage so I'm not concerned about it it though it does a lot.

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#39

Posted by joeymorin: Thu. Sep 14, 2017 - 02:51 PM

But:

theusch wrote:

??? I'm losing track a bit, but why race? Read the inputs per previous setup. Apply new setup for next multiplex period. Let multiplex period elapse. Rinse and repeat; no NOP in sight.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#40

Posted by alank2: Thu. Sep 14, 2017 - 03:14 PM

One of the reasons is that I am using a switch to figure out which row to work on and I figured another switch to split the sections up might be worse than the 8 cycles. Enclosing the code if anyone has tips/ideas.

    //lampboard and keyboard
    if (BrightTimer==ThisBright)
      {
        //disable rows
        DDR(LAMPBOARD_PORT)&=~(_BV(LAMPBOARD_ROW0_PIN) | _BV(LAMPBOARD_ROW1_PIN) | _BV(LAMPBOARD_ROW2_PIN) | _BV(LAMPBOARD_ROW3_PIN) | _BV(LAMPBOARD_ROW4_PIN) | _BV(LAMPBOARD_ROW5_PIN));
      }

    BrightTimer++;
    if (BrightTimer==LAMPBOARD_BRIGHT_LEVELS)
      {
        //keyboard input

        //columns input with pullup
        LBKBSHARED_PORT|=_BV(LBKBSHARED_COL0_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL4_PIN);
        DDR(LBKBSHARED_PORT)&=~(_BV(LBKBSHARED_COL0_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL4_PIN));

        //76543210 76543210 76543210 76543210
        // 0000011 11122222 33333444 4455555s

        #define SETTLE_DELAY 8

        switch(Row)
          {
            case 0:
              //row 0
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW0_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[0]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) >> 1;
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW0_PIN);
              break;

            case 1:
              //row 1
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW1_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[0]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN))) >> 6;
              ((uint8_t*)&AInput)[1]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) << 2;
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW1_PIN);
              break;

            case 2:
              //row 2
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW2_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[1]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) >> 3;
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW2_PIN);
              break;

            case 3:
              //row 3
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW3_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[2]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN)));
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW3_PIN);
              break;

            case 4:
              //row 4
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW4_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[2]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN))) >> 5;
              ((uint8_t*)&AInput)[3]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) << 3;
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW4_PIN);
              break;

            case 5:
              //row 5
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW5_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[3]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) >> 2;
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW5_PIN);
              if (~PIN(KEYBOARD_SEL_PORT) & _BV(KEYBOARD_SEL_PIN))
                ((uint8_t*)&AInput)[3]|=_BV(0);
              break;
            }

        //columns output
        DDR(LBKBSHARED_PORT)|=_BV(LBKBSHARED_COL0_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL4_PIN);

        BrightTimer=0;
        ThisBright=((Dimmed)?0:Configuration.LampboardBright);

        Row++;
        if (Row==6)
          {
            Row=0;

            //debounce
            if (InputLast==AInput)
              InputDebounceCount++;
            else
              {
                InputLast=AInput;
                InputDebounceCount=1;
              }

            //process into buffer
            if (InputState!=AInput && InputDebounceCount>=3)
              {
                //reset inactivity
                InactivityMilliseconds=0;
                InactivityMinutes=0;

                if (InputEventCount<KEYBOARD_BUFFER_SIZE)
                  {
                    InputBuffer[InputBufferIn]=AInput;
                    InputBufferIn++;
                    if (InputBufferIn==KEYBOARD_BUFFER_SIZE)
                      InputBufferIn=0;
                    InputEventCount++;
                  }

                //update inputstate
                InputState=AInput;
              }

            //inactivity
            InactivityMilliseconds++;
            if (InactivityMilliseconds==6250) //5000/6/8 * 60
              {
                InactivityMilliseconds=0;
                if (InactivityMinutes<65535)
                  InactivityMinutes++;
              }
          }

        if (~Lampboard[Row] & _BV(0))
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL0_PIN);
        if (~Lampboard[Row] & _BV(1))
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL1_PIN);
        if (~Lampboard[Row] & _BV(2))
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL2_PIN);
        if (~Lampboard[Row] & _BV(3))
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL3_PIN);
        if (~Lampboard[Row] & _BV(4))
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL4_PIN);

        switch(Row)
          {
            case 0 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW0_PIN); break;
            case 1 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW1_PIN); break;
            case 2 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW2_PIN); break;
            case 3 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW3_PIN); break;
            case 4 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW4_PIN); break;
            case 5 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW5_PIN); break;
          }
      }

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#41

Posted by alank2: Thu. Sep 14, 2017 - 03:27 PM

I suppose I could move:

        BrightTimer=0;
        ThisBright=((Dimmed)?0:Configuration.LampboardBright);

        Row++;

to where each delay is. I just hate to repeat code like that, but perhaps it is for the best. Can you do a function within a function for something like this?

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#42

Posted by joeymorin: Thu. Sep 14, 2017 - 03:37 PM

Can you do a function within a function for something like this?

No, but you can use a function-like macro. There could be (potentially, depending on how smart the optimiser is) duplication of executable code, but not of source code.

I have not reviewed your code, it's fairly difficult to follow... you seem to have an aversion to the space bar and the return key ;-) ... also, I've always found the _BV( macro to be less clear than the standard use of (1 <<

And there seems to be a lot of casting going on. Surely there's a clearer way to accomplish this?

It looks like you're debouncing each row (button?) individually...? Why? Better to debounce the entire matrix in one fell swoop. The scanning cycle can generate a 'scan code' which represents the combined bits read during each phase of the cycle. That single value is what you debounce.

The switch/case structure seems heavily dependent upon the choice of ports/bits for each row. 'Non-orthogonal'. A right pain to follow and debug, I'd say.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

Last Edited: Thu. Sep 14, 2017 - 03:48 PM

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#43

Posted by alank2: Thu. Sep 14, 2017 - 03:53 PM

The casting is to deal with a uint32_t (what holds the keyboard bits) as a uint8_t, otherwise the compiler goes nuts putting in tons of instructions. If I want to just alter some bits in that uin32_t that won't affect others, this is the method I've been using.

That is funny about (1 << x) - I greatly prefer the look of the _BV macro! I suppose everyone is different.

I am debouncing the entire uin32_t as one. See where it is being compared to InputLast.

I am stuffing groups of 5 bits into the 32 bit variable which is why I'm doing that.

All tips welcome! Thanks!

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#44

Posted by alank2: Thu. Sep 14, 2017 - 03:53 PM

I did move the 3 lines in to each case and eliminated the NOP's and it is working.

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#45

Posted by alank2: Thu. Sep 14, 2017 - 03:54 PM

Here is the LSS.

    //lampboard and keyboard
    if (BrightTimer==ThisBright)
    5546:	90 91 22 01 	lds	r25, 0x0122	; 0x800122 <BrightTimer.3756>
    554a:	80 91 21 01 	lds	r24, 0x0121	; 0x800121 <ThisBright.3757>
    554e:	98 13       	cpse	r25, r24
    5550:	03 c0       	rjmp	.+6      	; 0x5558 <__vector_16+0xd6>
      {
        //disable rows
        DDR(LAMPBOARD_PORT)&=~(_BV(LAMPBOARD_ROW0_PIN) | _BV(LAMPBOARD_ROW1_PIN) | _BV(LAMPBOARD_ROW2_PIN) | _BV(LAMPBOARD_ROW3_PIN) | _BV(LAMPBOARD_ROW4_PIN) | _BV(LAMPBOARD_ROW5_PIN));
    5552:	87 b1       	in	r24, 0x07	; 7
    5554:	83 70       	andi	r24, 0x03	; 3
    5556:	87 b9       	out	0x07, r24	; 7
      }

    BrightTimer++;
    5558:	80 91 22 01 	lds	r24, 0x0122	; 0x800122 <BrightTimer.3756>
    555c:	8f 5f       	subi	r24, 0xFF	; 255
    555e:	80 93 22 01 	sts	0x0122, r24	; 0x800122 <BrightTimer.3756>
    if (BrightTimer==LAMPBOARD_BRIGHT_LEVELS)
    5562:	88 30       	cpi	r24, 0x08	; 8
    5564:	09 f0       	breq	.+2      	; 0x5568 <__vector_16+0xe6>
    5566:	be c1       	rjmp	.+892    	; 0x58e4 <__vector_16+0x462>
      {
        //keyboard input

        //columns input with pullup
        LBKBSHARED_PORT|=_BV(LBKBSHARED_COL0_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL4_PIN);
    5568:	8b b1       	in	r24, 0x0b	; 11
    556a:	88 6f       	ori	r24, 0xF8	; 248
    556c:	8b b9       	out	0x0b, r24	; 11
        DDR(LBKBSHARED_PORT)&=~(_BV(LBKBSHARED_COL0_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL4_PIN));
    556e:	8a b1       	in	r24, 0x0a	; 10
    5570:	87 70       	andi	r24, 0x07	; 7
    5572:	8a b9       	out	0x0a, r24	; 10
        //76543210 76543210 76543210 76543210
        // 0000011 11122222 33333444 4455555s

        #define SETTLE_DELAY 8

        switch(Row)
    5574:	80 91 20 01 	lds	r24, 0x0120	; 0x800120 <Row.3755>
    5578:	82 30       	cpi	r24, 0x02	; 2
    557a:	09 f4       	brne	.+2      	; 0x557e <__vector_16+0xfc>
    557c:	54 c0       	rjmp	.+168    	; 0x5626 <__vector_16+0x1a4>
    557e:	28 f4       	brcc	.+10     	; 0x558a <__vector_16+0x108>
    5580:	88 23       	and	r24, r24
    5582:	51 f0       	breq	.+20     	; 0x5598 <__vector_16+0x116>
    5584:	81 30       	cpi	r24, 0x01	; 1
    5586:	21 f1       	breq	.+72     	; 0x55d0 <__vector_16+0x14e>
    5588:	b7 c0       	rjmp	.+366    	; 0x56f8 <__vector_16+0x276>
    558a:	83 30       	cpi	r24, 0x03	; 3
    558c:	09 f4       	brne	.+2      	; 0x5590 <__vector_16+0x10e>
    558e:	6f c0       	rjmp	.+222    	; 0x566e <__vector_16+0x1ec>
    5590:	84 30       	cpi	r24, 0x04	; 4
    5592:	09 f4       	brne	.+2      	; 0x5596 <__vector_16+0x114>
    5594:	85 c0       	rjmp	.+266    	; 0x56a0 <__vector_16+0x21e>
    5596:	b0 c0       	rjmp	.+352    	; 0x56f8 <__vector_16+0x276>
          {
            case 0:
              //row 0
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW0_PIN);
    5598:	08 9a       	sbi	0x01, 0	; 1
              //__builtin_avr_delay_cycles(SETTLE_DELAY);

              //these 3 lines here on purpose to use cpu cycles
              BrightTimer=0;
    559a:	10 92 22 01 	sts	0x0122, r1	; 0x800122 <BrightTimer.3756>
              ThisBright=((Dimmed)?0:Configuration.LampboardBright);
    559e:	90 91 1f 01 	lds	r25, 0x011F	; 0x80011f <Dimmed.3762>
    55a2:	91 11       	cpse	r25, r1
    55a4:	02 c0       	rjmp	.+4      	; 0x55aa <__vector_16+0x128>
    55a6:	80 91 c5 01 	lds	r24, 0x01C5	; 0x8001c5 <Configuration>
    55aa:	80 93 21 01 	sts	0x0121, r24	; 0x800121 <ThisBright.3757>
              Row++;
    55ae:	80 91 20 01 	lds	r24, 0x0120	; 0x800120 <Row.3755>
    55b2:	8f 5f       	subi	r24, 0xFF	; 255
    55b4:	80 93 20 01 	sts	0x0120, r24	; 0x800120 <Row.3755>

              ((uint8_t*)&AInput)[0]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) >> 1;
    55b8:	89 b1       	in	r24, 0x09	; 9
    55ba:	90 e0       	ldi	r25, 0x00	; 0
    55bc:	80 95       	com	r24
    55be:	90 95       	com	r25
    55c0:	88 7f       	andi	r24, 0xF8	; 248
    55c2:	99 27       	eor	r25, r25
    55c4:	95 95       	asr	r25
    55c6:	87 95       	ror	r24
    55c8:	80 93 1b 01 	sts	0x011B, r24	; 0x80011b <AInput.3770>
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW0_PIN);
    55cc:	08 98       	cbi	0x01, 0	; 1
              break;
    55ce:	bb c0       	rjmp	.+374    	; 0x5746 <__vector_16+0x2c4>

            case 1:
              //row 1
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW1_PIN);
    55d0:	09 9a       	sbi	0x01, 1	; 1
              //__builtin_avr_delay_cycles(SETTLE_DELAY);

              //these 3 lines here on purpose to use cpu cycles
              BrightTimer=0;
    55d2:	10 92 22 01 	sts	0x0122, r1	; 0x800122 <BrightTimer.3756>
              ThisBright=((Dimmed)?0:Configuration.LampboardBright);
    55d6:	80 91 1f 01 	lds	r24, 0x011F	; 0x80011f <Dimmed.3762>
    55da:	81 11       	cpse	r24, r1
    55dc:	03 c0       	rjmp	.+6      	; 0x55e4 <__vector_16+0x162>
    55de:	80 91 c5 01 	lds	r24, 0x01C5	; 0x8001c5 <Configuration>
    55e2:	01 c0       	rjmp	.+2      	; 0x55e6 <__vector_16+0x164>
    55e4:	80 e0       	ldi	r24, 0x00	; 0
    55e6:	80 93 21 01 	sts	0x0121, r24	; 0x800121 <ThisBright.3757>
              Row++;
    55ea:	80 91 20 01 	lds	r24, 0x0120	; 0x800120 <Row.3755>
    55ee:	8f 5f       	subi	r24, 0xFF	; 255
    55f0:	80 93 20 01 	sts	0x0120, r24	; 0x800120 <Row.3755>

              ((uint8_t*)&AInput)[0]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN))) >> 6;
    55f4:	89 b1       	in	r24, 0x09	; 9
    55f6:	90 e0       	ldi	r25, 0x00	; 0
    55f8:	80 95       	com	r24
    55fa:	90 95       	com	r25
    55fc:	80 7c       	andi	r24, 0xC0	; 192
    55fe:	99 27       	eor	r25, r25
    5600:	56 e0       	ldi	r21, 0x06	; 6
    5602:	95 95       	asr	r25
    5604:	87 95       	ror	r24
    5606:	5a 95       	dec	r21
    5608:	e1 f7       	brne	.-8      	; 0x5602 <__vector_16+0x180>
    560a:	90 91 1b 01 	lds	r25, 0x011B	; 0x80011b <AInput.3770>
    560e:	89 2b       	or	r24, r25
    5610:	80 93 1b 01 	sts	0x011B, r24	; 0x80011b <AInput.3770>
              ((uint8_t*)&AInput)[1]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) << 2;
    5614:	89 b1       	in	r24, 0x09	; 9
    5616:	80 95       	com	r24
    5618:	88 73       	andi	r24, 0x38	; 56
    561a:	88 0f       	add	r24, r24
    561c:	88 0f       	add	r24, r24
    561e:	80 93 1c 01 	sts	0x011C, r24	; 0x80011c <AInput.3770+0x1>
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW1_PIN);
    5622:	09 98       	cbi	0x01, 1	; 1
              break;
    5624:	90 c0       	rjmp	.+288    	; 0x5746 <__vector_16+0x2c4>

            case 2:
              //row 2
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW2_PIN);
    5626:	0a 9a       	sbi	0x01, 2	; 1
              //__builtin_avr_delay_cycles(SETTLE_DELAY);

              //these 3 lines here on purpose to use cpu cycles
              BrightTimer=0;
    5628:	10 92 22 01 	sts	0x0122, r1	; 0x800122 <BrightTimer.3756>
              ThisBright=((Dimmed)?0:Configuration.LampboardBright);
    562c:	80 91 1f 01 	lds	r24, 0x011F	; 0x80011f <Dimmed.3762>
    5630:	81 11       	cpse	r24, r1
    5632:	03 c0       	rjmp	.+6      	; 0x563a <__vector_16+0x1b8>
    5634:	80 91 c5 01 	lds	r24, 0x01C5	; 0x8001c5 <Configuration>
    5638:	01 c0       	rjmp	.+2      	; 0x563c <__vector_16+0x1ba>
    563a:	80 e0       	ldi	r24, 0x00	; 0
    563c:	80 93 21 01 	sts	0x0121, r24	; 0x800121 <ThisBright.3757>
              Row++;
    5640:	80 91 20 01 	lds	r24, 0x0120	; 0x800120 <Row.3755>
    5644:	8f 5f       	subi	r24, 0xFF	; 255
    5646:	80 93 20 01 	sts	0x0120, r24	; 0x800120 <Row.3755>

              ((uint8_t*)&AInput)[1]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) >> 3;
    564a:	89 b1       	in	r24, 0x09	; 9
    564c:	90 e0       	ldi	r25, 0x00	; 0
    564e:	80 95       	com	r24
    5650:	90 95       	com	r25
    5652:	88 7f       	andi	r24, 0xF8	; 248
    5654:	99 27       	eor	r25, r25
    5656:	43 e0       	ldi	r20, 0x03	; 3
    5658:	95 95       	asr	r25
    565a:	87 95       	ror	r24
    565c:	4a 95       	dec	r20
    565e:	e1 f7       	brne	.-8      	; 0x5658 <__vector_16+0x1d6>
    5660:	90 91 1c 01 	lds	r25, 0x011C	; 0x80011c <AInput.3770+0x1>
    5664:	89 2b       	or	r24, r25
    5666:	80 93 1c 01 	sts	0x011C, r24	; 0x80011c <AInput.3770+0x1>
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW2_PIN);
    566a:	0a 98       	cbi	0x01, 2	; 1
              break;
    566c:	6c c0       	rjmp	.+216    	; 0x5746 <__vector_16+0x2c4>

            case 3:
              //row 3
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW3_PIN);
    566e:	0b 9a       	sbi	0x01, 3	; 1
              //__builtin_avr_delay_cycles(SETTLE_DELAY);

              //these 3 lines here on purpose to use cpu cycles
              BrightTimer=0;
    5670:	10 92 22 01 	sts	0x0122, r1	; 0x800122 <BrightTimer.3756>
              ThisBright=((Dimmed)?0:Configuration.LampboardBright);
    5674:	80 91 1f 01 	lds	r24, 0x011F	; 0x80011f <Dimmed.3762>
    5678:	81 11       	cpse	r24, r1
    567a:	03 c0       	rjmp	.+6      	; 0x5682 <__vector_16+0x200>
    567c:	80 91 c5 01 	lds	r24, 0x01C5	; 0x8001c5 <Configuration>
    5680:	01 c0       	rjmp	.+2      	; 0x5684 <__vector_16+0x202>
    5682:	80 e0       	ldi	r24, 0x00	; 0
    5684:	80 93 21 01 	sts	0x0121, r24	; 0x800121 <ThisBright.3757>
              Row++;
    5688:	80 91 20 01 	lds	r24, 0x0120	; 0x800120 <Row.3755>
    568c:	8f 5f       	subi	r24, 0xFF	; 255
    568e:	80 93 20 01 	sts	0x0120, r24	; 0x800120 <Row.3755>

              ((uint8_t*)&AInput)[2]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN)));
    5692:	89 b1       	in	r24, 0x09	; 9
    5694:	80 95       	com	r24
    5696:	88 7f       	andi	r24, 0xF8	; 248
    5698:	80 93 1d 01 	sts	0x011D, r24	; 0x80011d <AInput.3770+0x2>
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW3_PIN);
    569c:	0b 98       	cbi	0x01, 3	; 1
              break;
    569e:	53 c0       	rjmp	.+166    	; 0x5746 <__vector_16+0x2c4>

            case 4:
              //row 4
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW4_PIN);
    56a0:	0c 9a       	sbi	0x01, 4	; 1
              //__builtin_avr_delay_cycles(SETTLE_DELAY);

              //these 3 lines here on purpose to use cpu cycles
              BrightTimer=0;
    56a2:	10 92 22 01 	sts	0x0122, r1	; 0x800122 <BrightTimer.3756>
              ThisBright=((Dimmed)?0:Configuration.LampboardBright);
    56a6:	80 91 1f 01 	lds	r24, 0x011F	; 0x80011f <Dimmed.3762>
    56aa:	81 11       	cpse	r24, r1
    56ac:	03 c0       	rjmp	.+6      	; 0x56b4 <__vector_16+0x232>
    56ae:	80 91 c5 01 	lds	r24, 0x01C5	; 0x8001c5 <Configuration>
    56b2:	01 c0       	rjmp	.+2      	; 0x56b6 <__vector_16+0x234>
    56b4:	80 e0       	ldi	r24, 0x00	; 0
    56b6:	80 93 21 01 	sts	0x0121, r24	; 0x800121 <ThisBright.3757>
              Row++;
    56ba:	80 91 20 01 	lds	r24, 0x0120	; 0x800120 <Row.3755>
    56be:	8f 5f       	subi	r24, 0xFF	; 255
    56c0:	80 93 20 01 	sts	0x0120, r24	; 0x800120 <Row.3755>

              ((uint8_t*)&AInput)[2]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN))) >> 5;
    56c4:	89 b1       	in	r24, 0x09	; 9
    56c6:	90 e0       	ldi	r25, 0x00	; 0
    56c8:	80 95       	com	r24
    56ca:	90 95       	com	r25
    56cc:	80 7e       	andi	r24, 0xE0	; 224
    56ce:	99 27       	eor	r25, r25
    56d0:	35 e0       	ldi	r19, 0x05	; 5
    56d2:	95 95       	asr	r25
    56d4:	87 95       	ror	r24
    56d6:	3a 95       	dec	r19
    56d8:	e1 f7       	brne	.-8      	; 0x56d2 <__vector_16+0x250>
    56da:	90 91 1d 01 	lds	r25, 0x011D	; 0x80011d <AInput.3770+0x2>
    56de:	89 2b       	or	r24, r25
    56e0:	80 93 1d 01 	sts	0x011D, r24	; 0x80011d <AInput.3770+0x2>
              ((uint8_t*)&AInput)[3]=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) << 3;
    56e4:	89 b1       	in	r24, 0x09	; 9
    56e6:	80 95       	com	r24
    56e8:	88 71       	andi	r24, 0x18	; 24
    56ea:	88 0f       	add	r24, r24
    56ec:	88 0f       	add	r24, r24
    56ee:	88 0f       	add	r24, r24
    56f0:	80 93 1e 01 	sts	0x011E, r24	; 0x80011e <AInput.3770+0x3>
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW4_PIN);
    56f4:	0c 98       	cbi	0x01, 4	; 1
              break;
    56f6:	27 c0       	rjmp	.+78     	; 0x5746 <__vector_16+0x2c4>

            default:
            //case 5:
              //row 5
              DDR(KEYBOARD_PORT)|=_BV(KEYBOARD_ROW5_PIN);
    56f8:	0d 9a       	sbi	0x01, 5	; 1
              //__builtin_avr_delay_cycles(SETTLE_DELAY);

              //these 3 lines here on purpose to use cpu cycles
              BrightTimer=0;
    56fa:	10 92 22 01 	sts	0x0122, r1	; 0x800122 <BrightTimer.3756>
              ThisBright=((Dimmed)?0:Configuration.LampboardBright);
    56fe:	80 91 1f 01 	lds	r24, 0x011F	; 0x80011f <Dimmed.3762>
    5702:	81 11       	cpse	r24, r1
    5704:	03 c0       	rjmp	.+6      	; 0x570c <__vector_16+0x28a>
    5706:	80 91 c5 01 	lds	r24, 0x01C5	; 0x8001c5 <Configuration>
    570a:	01 c0       	rjmp	.+2      	; 0x570e <__vector_16+0x28c>
    570c:	80 e0       	ldi	r24, 0x00	; 0
    570e:	80 93 21 01 	sts	0x0121, r24	; 0x800121 <ThisBright.3757>
              Row++;
    5712:	80 91 20 01 	lds	r24, 0x0120	; 0x800120 <Row.3755>
    5716:	8f 5f       	subi	r24, 0xFF	; 255
    5718:	80 93 20 01 	sts	0x0120, r24	; 0x800120 <Row.3755>

              ((uint8_t*)&AInput)[3]|=(~PIN(LBKBSHARED_PORT) & (_BV(LBKBSHARED_COL4_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL0_PIN))) >> 2;
    571c:	89 b1       	in	r24, 0x09	; 9
    571e:	90 e0       	ldi	r25, 0x00	; 0
    5720:	80 95       	com	r24
    5722:	90 95       	com	r25
    5724:	88 7f       	andi	r24, 0xF8	; 248
    5726:	99 27       	eor	r25, r25
    5728:	95 95       	asr	r25
    572a:	87 95       	ror	r24
    572c:	95 95       	asr	r25
    572e:	87 95       	ror	r24
    5730:	90 91 1e 01 	lds	r25, 0x011E	; 0x80011e <AInput.3770+0x3>
    5734:	89 2b       	or	r24, r25
    5736:	80 93 1e 01 	sts	0x011E, r24	; 0x80011e <AInput.3770+0x3>
              DDR(KEYBOARD_PORT)&=~_BV(KEYBOARD_ROW5_PIN);
    573a:	0d 98       	cbi	0x01, 5	; 1
              if (~PIN(KEYBOARD_SEL_PORT) & _BV(KEYBOARD_SEL_PIN))
    573c:	4a 99       	sbic	0x09, 2	; 9
    573e:	03 c0       	rjmp	.+6      	; 0x5746 <__vector_16+0x2c4>
                ((uint8_t*)&AInput)[3]|=_BV(0);
    5740:	81 60       	ori	r24, 0x01	; 1
    5742:	80 93 1e 01 	sts	0x011E, r24	; 0x80011e <AInput.3770+0x3>
              break;
            }

        //columns output
        DDR(LBKBSHARED_PORT)|=_BV(LBKBSHARED_COL0_PIN) | _BV(LBKBSHARED_COL1_PIN) | _BV(LBKBSHARED_COL2_PIN) | _BV(LBKBSHARED_COL3_PIN) | _BV(LBKBSHARED_COL4_PIN);
    5746:	8a b1       	in	r24, 0x0a	; 10
    5748:	88 6f       	ori	r24, 0xF8	; 248
    574a:	8a b9       	out	0x0a, r24	; 10
        
        /*BrightTimer=0;
        ThisBright=((Dimmed)?0:Configuration.LampboardBright);
        Row++;*/

        if (Row==6)
    574c:	80 91 20 01 	lds	r24, 0x0120	; 0x800120 <Row.3755>
    5750:	86 30       	cpi	r24, 0x06	; 6
    5752:	09 f0       	breq	.+2      	; 0x5756 <__vector_16+0x2d4>
    5754:	86 c0       	rjmp	.+268    	; 0x5862 <__vector_16+0x3e0>
          {
            Row=0;
    5756:	10 92 20 01 	sts	0x0120, r1	; 0x800120 <Row.3755>

            //debounce
            if (InputLast==AInput)
    575a:	80 91 1b 01 	lds	r24, 0x011B	; 0x80011b <AInput.3770>
    575e:	90 91 1c 01 	lds	r25, 0x011C	; 0x80011c <AInput.3770+0x1>
    5762:	a0 91 1d 01 	lds	r26, 0x011D	; 0x80011d <AInput.3770+0x2>
    5766:	b0 91 1e 01 	lds	r27, 0x011E	; 0x80011e <AInput.3770+0x3>
    576a:	40 91 17 01 	lds	r20, 0x0117	; 0x800117 <InputLast.3769>
    576e:	50 91 18 01 	lds	r21, 0x0118	; 0x800118 <InputLast.3769+0x1>
    5772:	60 91 19 01 	lds	r22, 0x0119	; 0x800119 <InputLast.3769+0x2>
    5776:	70 91 1a 01 	lds	r23, 0x011A	; 0x80011a <InputLast.3769+0x3>
    577a:	48 17       	cp	r20, r24
    577c:	59 07       	cpc	r21, r25
    577e:	6a 07       	cpc	r22, r26
    5780:	7b 07       	cpc	r23, r27
    5782:	21 f4       	brne	.+8      	; 0x578c <__vector_16+0x30a>
              InputDebounceCount++;
    5784:	20 91 16 01 	lds	r18, 0x0116	; 0x800116 <InputDebounceCount.3758>
    5788:	2f 5f       	subi	r18, 0xFF	; 255
    578a:	09 c0       	rjmp	.+18     	; 0x579e <__vector_16+0x31c>
            else
              {
                InputLast=AInput;
    578c:	80 93 17 01 	sts	0x0117, r24	; 0x800117 <InputLast.3769>
    5790:	90 93 18 01 	sts	0x0118, r25	; 0x800118 <InputLast.3769+0x1>
    5794:	a0 93 19 01 	sts	0x0119, r26	; 0x800119 <InputLast.3769+0x2>
    5798:	b0 93 1a 01 	sts	0x011A, r27	; 0x80011a <InputLast.3769+0x3>
                InputDebounceCount=1;
    579c:	21 e0       	ldi	r18, 0x01	; 1
    579e:	20 93 16 01 	sts	0x0116, r18	; 0x800116 <InputDebounceCount.3758>
              }

            //process into buffer
            if (InputState!=AInput && InputDebounceCount>=3)
    57a2:	40 91 ba 01 	lds	r20, 0x01BA	; 0x8001ba <InputState>
    57a6:	50 91 bb 01 	lds	r21, 0x01BB	; 0x8001bb <InputState+0x1>
    57aa:	60 91 bc 01 	lds	r22, 0x01BC	; 0x8001bc <InputState+0x2>
    57ae:	70 91 bd 01 	lds	r23, 0x01BD	; 0x8001bd <InputState+0x3>
    57b2:	48 17       	cp	r20, r24
    57b4:	59 07       	cpc	r21, r25
    57b6:	6a 07       	cpc	r22, r26
    57b8:	7b 07       	cpc	r23, r27
    57ba:	a1 f1       	breq	.+104    	; 0x5824 <__vector_16+0x3a2>
    57bc:	20 91 16 01 	lds	r18, 0x0116	; 0x800116 <InputDebounceCount.3758>
    57c0:	23 30       	cpi	r18, 0x03	; 3
    57c2:	80 f1       	brcs	.+96     	; 0x5824 <__vector_16+0x3a2>
              {
                //reset inactivity
                InactivityMilliseconds=0;
    57c4:	10 92 15 01 	sts	0x0115, r1	; 0x800115 <InactivityMilliseconds.3766+0x1>
    57c8:	10 92 14 01 	sts	0x0114, r1	; 0x800114 <InactivityMilliseconds.3766>
                InactivityMinutes=0;
    57cc:	10 92 13 01 	sts	0x0113, r1	; 0x800113 <InactivityMinutes.3767+0x1>
    57d0:	10 92 12 01 	sts	0x0112, r1	; 0x800112 <InactivityMinutes.3767>

                if (InputEventCount<KEYBOARD_BUFFER_SIZE)
    57d4:	20 91 be 01 	lds	r18, 0x01BE	; 0x8001be <InputEventCount>
    57d8:	20 32       	cpi	r18, 0x20	; 32
    57da:	e0 f4       	brcc	.+56     	; 0x5814 <__vector_16+0x392>
                  {
                    InputBuffer[InputBufferIn]=AInput;
    57dc:	e0 91 71 03 	lds	r30, 0x0371	; 0x800371 <InputBufferIn>
    57e0:	34 e0       	ldi	r19, 0x04	; 4
    57e2:	e3 9f       	mul	r30, r19
    57e4:	f0 01       	movw	r30, r0
    57e6:	11 24       	eor	r1, r1
    57e8:	e2 5d       	subi	r30, 0xD2	; 210
    57ea:	fe 4f       	sbci	r31, 0xFE	; 254
    57ec:	80 83       	st	Z, r24
    57ee:	91 83       	std	Z+1, r25	; 0x01
    57f0:	a2 83       	std	Z+2, r26	; 0x02
    57f2:	b3 83       	std	Z+3, r27	; 0x03
                    InputBufferIn++;
    57f4:	20 91 71 03 	lds	r18, 0x0371	; 0x800371 <InputBufferIn>
    57f8:	2f 5f       	subi	r18, 0xFF	; 255
    57fa:	20 93 71 03 	sts	0x0371, r18	; 0x800371 <InputBufferIn>
                    if (InputBufferIn==KEYBOARD_BUFFER_SIZE)
    57fe:	20 91 71 03 	lds	r18, 0x0371	; 0x800371 <InputBufferIn>
    5802:	20 32       	cpi	r18, 0x20	; 32
    5804:	11 f4       	brne	.+4      	; 0x580a <__vector_16+0x388>
                      InputBufferIn=0;
    5806:	10 92 71 03 	sts	0x0371, r1	; 0x800371 <InputBufferIn>
                    InputEventCount++;
    580a:	20 91 be 01 	lds	r18, 0x01BE	; 0x8001be <InputEventCount>
    580e:	2f 5f       	subi	r18, 0xFF	; 255
    5810:	20 93 be 01 	sts	0x01BE, r18	; 0x8001be <InputEventCount>
                  }

                //update inputstate
                InputState=AInput;
    5814:	80 93 ba 01 	sts	0x01BA, r24	; 0x8001ba <InputState>
    5818:	90 93 bb 01 	sts	0x01BB, r25	; 0x8001bb <InputState+0x1>
    581c:	a0 93 bc 01 	sts	0x01BC, r26	; 0x8001bc <InputState+0x2>
    5820:	b0 93 bd 01 	sts	0x01BD, r27	; 0x8001bd <InputState+0x3>
              }

            //inactivity
            InactivityMilliseconds++;
    5824:	80 91 14 01 	lds	r24, 0x0114	; 0x800114 <InactivityMilliseconds.3766>
    5828:	90 91 15 01 	lds	r25, 0x0115	; 0x800115 <InactivityMilliseconds.3766+0x1>
    582c:	01 96       	adiw	r24, 0x01	; 1
            if (InactivityMilliseconds==6250) //5000/6/8 * 60
    582e:	8a 36       	cpi	r24, 0x6A	; 106
    5830:	e8 e1       	ldi	r30, 0x18	; 24
    5832:	9e 07       	cpc	r25, r30
    5834:	29 f0       	breq	.+10     	; 0x5840 <__vector_16+0x3be>
                //update inputstate
                InputState=AInput;
              }

            //inactivity
            InactivityMilliseconds++;
    5836:	90 93 15 01 	sts	0x0115, r25	; 0x800115 <InactivityMilliseconds.3766+0x1>
    583a:	80 93 14 01 	sts	0x0114, r24	; 0x800114 <InactivityMilliseconds.3766>
    583e:	11 c0       	rjmp	.+34     	; 0x5862 <__vector_16+0x3e0>
            if (InactivityMilliseconds==6250) //5000/6/8 * 60
              {
                InactivityMilliseconds=0;
    5840:	10 92 15 01 	sts	0x0115, r1	; 0x800115 <InactivityMilliseconds.3766+0x1>
    5844:	10 92 14 01 	sts	0x0114, r1	; 0x800114 <InactivityMilliseconds.3766>
                if (InactivityMinutes<65535)
    5848:	80 91 12 01 	lds	r24, 0x0112	; 0x800112 <InactivityMinutes.3767>
    584c:	90 91 13 01 	lds	r25, 0x0113	; 0x800113 <InactivityMinutes.3767+0x1>
    5850:	8f 3f       	cpi	r24, 0xFF	; 255
    5852:	ff ef       	ldi	r31, 0xFF	; 255
    5854:	9f 07       	cpc	r25, r31
    5856:	29 f0       	breq	.+10     	; 0x5862 <__vector_16+0x3e0>
                  InactivityMinutes++;
    5858:	01 96       	adiw	r24, 0x01	; 1
    585a:	90 93 13 01 	sts	0x0113, r25	; 0x800113 <InactivityMinutes.3767+0x1>
    585e:	80 93 12 01 	sts	0x0112, r24	; 0x800112 <InactivityMinutes.3767>
              }
          }

        if (~Lampboard[Row] & _BV(0))
    5862:	e0 91 20 01 	lds	r30, 0x0120	; 0x800120 <Row.3755>
    5866:	f0 e0       	ldi	r31, 0x00	; 0
    5868:	e1 54       	subi	r30, 0x41	; 65
    586a:	fe 4f       	sbci	r31, 0xFE	; 254
    586c:	80 81       	ld	r24, Z
    586e:	80 ff       	sbrs	r24, 0
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL0_PIN);
    5870:	5b 98       	cbi	0x0b, 3	; 11
        if (~Lampboard[Row] & _BV(1))
    5872:	e0 91 20 01 	lds	r30, 0x0120	; 0x800120 <Row.3755>
    5876:	f0 e0       	ldi	r31, 0x00	; 0
    5878:	e1 54       	subi	r30, 0x41	; 65
    587a:	fe 4f       	sbci	r31, 0xFE	; 254
    587c:	80 81       	ld	r24, Z
    587e:	81 ff       	sbrs	r24, 1
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL1_PIN);
    5880:	5c 98       	cbi	0x0b, 4	; 11
        if (~Lampboard[Row] & _BV(2))
    5882:	e0 91 20 01 	lds	r30, 0x0120	; 0x800120 <Row.3755>
    5886:	f0 e0       	ldi	r31, 0x00	; 0
    5888:	e1 54       	subi	r30, 0x41	; 65
    588a:	fe 4f       	sbci	r31, 0xFE	; 254
    588c:	80 81       	ld	r24, Z
    588e:	82 ff       	sbrs	r24, 2
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL2_PIN);
    5890:	5d 98       	cbi	0x0b, 5	; 11
        if (~Lampboard[Row] & _BV(3))
    5892:	e0 91 20 01 	lds	r30, 0x0120	; 0x800120 <Row.3755>
    5896:	f0 e0       	ldi	r31, 0x00	; 0
    5898:	e1 54       	subi	r30, 0x41	; 65
    589a:	fe 4f       	sbci	r31, 0xFE	; 254
    589c:	80 81       	ld	r24, Z
    589e:	83 ff       	sbrs	r24, 3
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL3_PIN);
    58a0:	5e 98       	cbi	0x0b, 6	; 11
        if (~Lampboard[Row] & _BV(4))
    58a2:	e0 91 20 01 	lds	r30, 0x0120	; 0x800120 <Row.3755>
    58a6:	f0 e0       	ldi	r31, 0x00	; 0
    58a8:	e1 54       	subi	r30, 0x41	; 65
    58aa:	fe 4f       	sbci	r31, 0xFE	; 254
    58ac:	80 81       	ld	r24, Z
    58ae:	84 ff       	sbrs	r24, 4
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL4_PIN);
    58b0:	5f 98       	cbi	0x0b, 7	; 11

        switch(Row)
    58b2:	80 91 20 01 	lds	r24, 0x0120	; 0x800120 <Row.3755>
    58b6:	82 30       	cpi	r24, 0x02	; 2
    58b8:	81 f0       	breq	.+32     	; 0x58da <__vector_16+0x458>
    58ba:	30 f4       	brcc	.+12     	; 0x58c8 <__vector_16+0x446>
    58bc:	88 23       	and	r24, r24
    58be:	59 f0       	breq	.+22     	; 0x58d6 <__vector_16+0x454>
    58c0:	81 30       	cpi	r24, 0x01	; 1
    58c2:	81 f4       	brne	.+32     	; 0x58e4 <__vector_16+0x462>
          {
            case 0 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW0_PIN); break;
            case 1 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW1_PIN); break;
    58c4:	3b 9a       	sbi	0x07, 3	; 7
    58c6:	0e c0       	rjmp	.+28     	; 0x58e4 <__vector_16+0x462>
        if (~Lampboard[Row] & _BV(3))
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL3_PIN);
        if (~Lampboard[Row] & _BV(4))
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL4_PIN);

        switch(Row)
    58c8:	84 30       	cpi	r24, 0x04	; 4
    58ca:	59 f0       	breq	.+22     	; 0x58e2 <__vector_16+0x460>
    58cc:	40 f0       	brcs	.+16     	; 0x58de <__vector_16+0x45c>
    58ce:	85 30       	cpi	r24, 0x05	; 5
    58d0:	49 f4       	brne	.+18     	; 0x58e4 <__vector_16+0x462>
            case 0 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW0_PIN); break;
            case 1 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW1_PIN); break;
            case 2 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW2_PIN); break;
            case 3 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW3_PIN); break;
            case 4 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW4_PIN); break;
            case 5 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW5_PIN); break;
    58d2:	3f 9a       	sbi	0x07, 7	; 7
    58d4:	07 c0       	rjmp	.+14     	; 0x58e4 <__vector_16+0x462>
        if (~Lampboard[Row] & _BV(4))
          LBKBSHARED_PORT&=~_BV(LBKBSHARED_COL4_PIN);

        switch(Row)
          {
            case 0 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW0_PIN); break;
    58d6:	3a 9a       	sbi	0x07, 2	; 7
    58d8:	05 c0       	rjmp	.+10     	; 0x58e4 <__vector_16+0x462>
            case 1 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW1_PIN); break;
            case 2 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW2_PIN); break;
    58da:	3c 9a       	sbi	0x07, 4	; 7
    58dc:	03 c0       	rjmp	.+6      	; 0x58e4 <__vector_16+0x462>
            case 3 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW3_PIN); break;
    58de:	3d 9a       	sbi	0x07, 5	; 7
    58e0:	01 c0       	rjmp	.+2      	; 0x58e4 <__vector_16+0x462>
            case 4 : DDR(LAMPBOARD_PORT)|=_BV(LAMPBOARD_ROW4_PIN); break;
    58e2:	3e 9a       	sbi	0x07, 6	; 7
      }

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#46

Posted by joeymorin: Thu. Sep 14, 2017 - 03:57 PM

Here's your code formatted to my liking (as if that matters ;-):

    // lampboard and keyboard
    if (BrightTimer == ThisBright)
      {
        // disable rows
        DDR(LAMPBOARD_PORT) &= ~( (1 << LAMPBOARD_ROW0_PIN) |
                                  (1 << LAMPBOARD_ROW1_PIN) |
                                  (1 << LAMPBOARD_ROW2_PIN) |
                                  (1 << LAMPBOARD_ROW3_PIN) |
                                  (1 << LAMPBOARD_ROW4_PIN) |
                                  (1 << LAMPBOARD_ROW5_PIN) );
      }

    BrightTimer++;
    if (BrightTimer == LAMPBOARD_BRIGHT_LEVELS)
      {
        // keyboard input

        // columns input with pullup
        LBKBSHARED_PORT |= (1 << LBKBSHARED_COL0_PIN) |
                           (1 << LBKBSHARED_COL1_PIN) |
                           (1 << LBKBSHARED_COL2_PIN) |
                           (1 << LBKBSHARED_COL3_PIN) |
                           (1 << LBKBSHARED_COL4_PIN);
        DDR(LBKBSHARED_PORT) &= ~( (1 << LBKBSHARED_COL0_PIN) |
                                   (1 << LBKBSHARED_COL1_PIN) |
                                   (1 << LBKBSHARED_COL2_PIN) |
                                   (1 << LBKBSHARED_COL3_PIN) |
                                   (1 << LBKBSHARED_COL4_PIN) );

        // 76543210 76543210 76543210 76543210
        //  0000011 11122222 33333444 4455555s

        #define SETTLE_DELAY 8

        switch(Row)
          {
            case 0:
              //row 0
              DDR(KEYBOARD_PORT)     |= (1 << KEYBOARD_ROW0_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[0]  = (~PIN(LBKBSHARED_PORT) &
                                          ( (1 << LBKBSHARED_COL4_PIN) |
                                            (1 << LBKBSHARED_COL3_PIN) |
                                            (1 << LBKBSHARED_COL2_PIN) |
                                            (1 << LBKBSHARED_COL1_PIN) |
                                            (1 << LBKBSHARED_COL0_PIN) ) ) >> 1;
              DDR(KEYBOARD_PORT)     &= ~(1 << KEYBOARD_ROW0_PIN);
              break;

            case 1:
              // row 1
              DDR(KEYBOARD_PORT)     |= (1 << KEYBOARD_ROW1_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[0] |= (~PIN(LBKBSHARED_PORT) &
                                          ( (1 << LBKBSHARED_COL4_PIN) |
                                            (1 << LBKBSHARED_COL3_PIN) ) ) >> 6;
              ((uint8_t*)&AInput)[1]  = (~PIN(LBKBSHARED_PORT) &
                                          ( (1 << LBKBSHARED_COL2_PIN) |
                                            (1 << LBKBSHARED_COL1_PIN) |
                                            (1 << LBKBSHARED_COL0_PIN) ) ) << 2;
              DDR(KEYBOARD_PORT)     &= ~(1 << KEYBOARD_ROW1_PIN);
              break;

            case 2:
              // row 2
              DDR(KEYBOARD_PORT)     |= (1 << KEYBOARD_ROW2_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[1] |= (~PIN(LBKBSHARED_PORT) &
                                          ( (1 << LBKBSHARED_COL4_PIN) |
                                            (1 << LBKBSHARED_COL3_PIN) |
                                            (1 << LBKBSHARED_COL2_PIN) |
                                            (1 << LBKBSHARED_COL1_PIN) |
                                            (1 << LBKBSHARED_COL0_PIN) ) ) >> 3;
              DDR(KEYBOARD_PORT)     &= ~(1 << KEYBOARD_ROW2_PIN);
              break;

            case 3:
              //row 3
              DDR(KEYBOARD_PORT)     |= (1 << KEYBOARD_ROW3_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[2]  = (~PIN(LBKBSHARED_PORT) &
                                          ( (1 << LBKBSHARED_COL4_PIN) |
                                            (1 << LBKBSHARED_COL3_PIN) |
                                            (1 << LBKBSHARED_COL2_PIN) |
                                            (1 << LBKBSHARED_COL1_PIN) |
                                            (1 << LBKBSHARED_COL0_PIN) ) );
              DDR(KEYBOARD_PORT)     &= ~(1 << KEYBOARD_ROW3_PIN);
              break;

            case 4:
              // row 4
              DDR(KEYBOARD_PORT)     |= (1 << KEYBOARD_ROW4_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[2] |= (~PIN(LBKBSHARED_PORT) &
                                          ( (1 << LBKBSHARED_COL4_PIN) |
                                            (1 << LBKBSHARED_COL3_PIN) |
                                            (1 << LBKBSHARED_COL2_PIN) ) ) >> 5;
              ((uint8_t*)&AInput)[3]  = (~PIN(LBKBSHARED_PORT) &
                                          ( (1 << LBKBSHARED_COL1_PIN) |
                                            (1 << LBKBSHARED_COL0_PIN) ) ) << 3;
              DDR(KEYBOARD_PORT)     &= ~(1 << KEYBOARD_ROW4_PIN);
              break;

            case 5:
              // row 5
              DDR(KEYBOARD_PORT)     |= (1 << KEYBOARD_ROW5_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[3] |= (~PIN(LBKBSHARED_PORT) &
                                          ( (1 << LBKBSHARED_COL4_PIN) |
                                            (1 << LBKBSHARED_COL3_PIN) |
                                            (1 << LBKBSHARED_COL2_PIN) |
                                            (1 << LBKBSHARED_COL1_PIN) |
                                            (1 << LBKBSHARED_COL0_PIN) ) ) >> 2;
              DDR(KEYBOARD_PORT)     &= ~(1 << KEYBOARD_ROW5_PIN);
              if (~PIN(KEYBOARD_SEL_PORT) & (1 << KEYBOARD_SEL_PIN))
                {
                  ((uint8_t*)&AInput)[3] |= (1 << 0);
                }
              break;
            }

        // columns output
        DDR(LBKBSHARED_PORT) |= (1 << LBKBSHARED_COL0_PIN) |
                                (1 << LBKBSHARED_COL1_PIN) |
                                (1 << LBKBSHARED_COL2_PIN) |
                                (1 << LBKBSHARED_COL3_PIN) |
                                (1 << LBKBSHARED_COL4_PIN);

        BrightTimer = 0;
        ThisBright  = Dimmed
                    ? 0
                    : Configuration.LampboardBright;

        Row++;
        if (Row == 6)
          {
            Row = 0;

            // debounce
            if (InputLast == AInput)
              {
                InputDebounceCount++;
              }
            else
              {
                InputLast          = AInput;
                InputDebounceCount = 1;
              }

            // process into buffer
            if ((InputState != AInput) && (InputDebounceCount >= 3))
              {
                // reset inactivity
                InactivityMilliseconds = 0;
                InactivityMinutes      = 0;

                if (InputEventCount < KEYBOARD_BUFFER_SIZE)
                  {
                    InputBuffer[InputBufferIn] = AInput;
                    InputBufferIn++;
                    if (InputBufferIn == KEYBOARD_BUFFER_SIZE)
                      {
                        InputBufferIn = 0;
                      }
                    InputEventCount++;
                  }

                // update inputstate
                InputState = AInput;
              }

            // inactivity
            InactivityMilliseconds++;
            if (InactivityMilliseconds == 6250) //5000/6/8 * 60
              {
                InactivityMilliseconds = 0;
                if (InactivityMinutes < 65535)
                  InactivityMinutes++;
              }
          }

        if (~Lampboard[Row] & (1 << 0))
          {
            LBKBSHARED_PORT &= ~(1 << LBKBSHARED_COL0_PIN);
          }
        if (~Lampboard[Row] & (1 << 1))
          {
            LBKBSHARED_PORT &= ~(1 << LBKBSHARED_COL1_PIN);
          }
        if (~Lampboard[Row] & (1 << 2))
          {
            LBKBSHARED_PORT &= ~(1 << LBKBSHARED_COL2_PIN);
          }
        if (~Lampboard[Row] & (1 << 3))
          {
            LBKBSHARED_PORT &= ~(1 << LBKBSHARED_COL3_PIN);
          }
        if (~Lampboard[Row] & (1 << 4))
          {
            LBKBSHARED_PORT &= ~(1 << LBKBSHARED_COL4_PIN);
          }

        switch(Row)
          {
            case 0 : DDR(LAMPBOARD_PORT) |= (1 << LAMPBOARD_ROW0_PIN); break;
            case 1 : DDR(LAMPBOARD_PORT) |= (1 << LAMPBOARD_ROW1_PIN); break;
            case 2 : DDR(LAMPBOARD_PORT) |= (1 << LAMPBOARD_ROW2_PIN); break;
            case 3 : DDR(LAMPBOARD_PORT) |= (1 << LAMPBOARD_ROW3_PIN); break;
            case 4 : DDR(LAMPBOARD_PORT) |= (1 << LAMPBOARD_ROW4_PIN); break;
            case 5 : DDR(LAMPBOARD_PORT) |= (1 << LAMPBOARD_ROW5_PIN); break;
          }
      }

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#47

Posted by joeymorin: Thu. Sep 14, 2017 - 04:17 PM

I am stuffing groups of 5 bits into the 32 bit variable which is why I'm doing that.

But why work with a 32bit value, when you have to stuff 5-bit values into it, often across byte boundaries? It makes for incredibly unclear code.

Your comments are a bit confusing:

        // 76543210 76543210 76543210 76543210
        //  0000011 11122222 33333444 4455555s

It's not clear if the LSByte is on the left or the right. Your code suggests the LSByte is on the left, with contradicts the bit order suggested by '76543210':

        switch(Row)
          {
            case 0:
              //row 0
              DDR(KEYBOARD_PORT)     |= (1 << KEYBOARD_ROW0_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[0]  = (~PIN(LBKBSHARED_PORT) &

So a more appropriate comment might be:

        // 76543210 76543210 76543210 76543210
        // s5555544 44433333 22222111 1100000

I assume you've padded by one bit to minimise the number of rows which cross byte boundaries?

In any event, use bitfields instead.

$ cat alank2.c
typedef struct {
  unsigned      : 1;
  unsigned row0 : 5;
  unsigned row1 : 5;
  unsigned row2 : 5;
  unsigned row3 : 5;
  unsigned row4 : 5;
  unsigned row5 : 5;
} foo_t;

static volatile foo_t foo;

int main(void) {

  foo.row0 = 0x15;
  foo.row1 = 0x15;
  foo.row2 = 0x15;
  foo.row3 = 0x15;
  foo.row4 = 0x15;
  foo.row5 = 0x15;

  while (1);

}

$ avr-gcc -Wall -Werror -O3 -funsigned-bitfields -fpack-struct -g alank2.c -o alank2.elf
$ avr-objdump -S alank2.elf 

alank2.elf:     file format elf32-avr

Disassembly of section .text:

00000000 <__ctors_end>:
   0:	20 e0       	ldi	r18, 0x00	; 0
   2:	a0 e6       	ldi	r26, 0x60	; 96
   4:	b0 e0       	ldi	r27, 0x00	; 0
   6:	01 c0       	rjmp	.+2      	; 0xa <.do_clear_bss_start>

00000008 <.do_clear_bss_loop>:
   8:	1d 92       	st	X+, r1

0000000a <.do_clear_bss_start>:
   a:	a4 36       	cpi	r26, 0x64	; 100
   c:	b2 07       	cpc	r27, r18
   e:	e1 f7       	brne	.-8      	; 0x8 <.do_clear_bss_loop>

00000010 <main>:

static volatile foo_t foo;

int main(void) {

  foo.row0 = 0x15;
  10:	80 91 60 00 	lds	r24, 0x0060	; 0x800060 <_edata>
  14:	81 7c       	andi	r24, 0xC1	; 193
  16:	8a 62       	ori	r24, 0x2A	; 42
  18:	80 93 60 00 	sts	0x0060, r24	; 0x800060 <_edata>
  foo.row1 = 0x15;
  1c:	80 91 60 00 	lds	r24, 0x0060	; 0x800060 <_edata>
  20:	8f 73       	andi	r24, 0x3F	; 63
  22:	80 64       	ori	r24, 0x40	; 64
  24:	80 93 60 00 	sts	0x0060, r24	; 0x800060 <_edata>
  28:	80 91 61 00 	lds	r24, 0x0061	; 0x800061 <_edata+0x1>
  2c:	88 7f       	andi	r24, 0xF8	; 248
  2e:	85 60       	ori	r24, 0x05	; 5
  30:	80 93 61 00 	sts	0x0061, r24	; 0x800061 <_edata+0x1>
  foo.row2 = 0x15;
  34:	80 91 61 00 	lds	r24, 0x0061	; 0x800061 <_edata+0x1>
  38:	87 70       	andi	r24, 0x07	; 7
  3a:	88 6a       	ori	r24, 0xA8	; 168
  3c:	80 93 61 00 	sts	0x0061, r24	; 0x800061 <_edata+0x1>
  foo.row3 = 0x15;
  40:	80 91 62 00 	lds	r24, 0x0062	; 0x800062 <_edata+0x2>
  44:	80 7e       	andi	r24, 0xE0	; 224
  46:	85 61       	ori	r24, 0x15	; 21
  48:	80 93 62 00 	sts	0x0062, r24	; 0x800062 <_edata+0x2>
  foo.row4 = 0x15;
  4c:	80 91 62 00 	lds	r24, 0x0062	; 0x800062 <_edata+0x2>
  50:	8f 71       	andi	r24, 0x1F	; 31
  52:	80 6a       	ori	r24, 0xA0	; 160
  54:	80 93 62 00 	sts	0x0062, r24	; 0x800062 <_edata+0x2>
  58:	80 91 63 00 	lds	r24, 0x0063	; 0x800063 <_edata+0x3>
  5c:	8c 7f       	andi	r24, 0xFC	; 252
  5e:	82 60       	ori	r24, 0x02	; 2
  60:	80 93 63 00 	sts	0x0063, r24	; 0x800063 <_edata+0x3>
  foo.row5 = 0x15;
  64:	80 91 63 00 	lds	r24, 0x0063	; 0x800063 <_edata+0x3>
  68:	83 78       	andi	r24, 0x83	; 131
  6a:	84 65       	ori	r24, 0x54	; 84
  6c:	80 93 63 00 	sts	0x0063, r24	; 0x800063 <_edata+0x3>

  while (1);
  70:	ff cf       	rjmp	.-2      	; 0x70 <__SREG__+0x31>

As you can see, the compiler generates fairly efficient code. And your source code will become much clearer. No need for casting, no need for arrays, no need for multiple expressions to cross byte boundaries. Let the compiler do that.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

Last Edited: Thu. Sep 14, 2017 - 04:45 PM

alank2

Level: Raving Lunatic

Joined: Thu. Jul 16, 2009

Posts: 2828 View posts

#48

Posted by alank2: Thu. Sep 14, 2017 - 04:44 PM

Thank you JM - I appreciate that. I haven't used bitfields before - this looks like a good reason to learn!

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#49

Posted by joeymorin: Thu. Sep 14, 2017 - 04:52 PM

You can still treat the whole struct as a single entity when comparing it for debouncing or against a specific keycode. This can be done either by casting the struct to, say, uint32_t, by use of a union, or by use of memcmp() and friends.

Either way, note that you would want to specify the value of the padded bits, and to ensure that there are no unspecified bits remaining:

typedef struct {
  unsigned pad0 : 1;
  unsigned row0 : 5;
  unsigned row1 : 5;
  unsigned row2 : 5;
  unsigned row3 : 5;
  unsigned row4 : 5;
  unsigned row5 : 5;
  unsigned pad1 : 1;
} foo_t;

static volatile foo_t foo = { .pad0 = 0, .pad1 = 0 };

Placing foo in .bss is sufficient to ensure all fields are initialised to 0, but explicitly naming all fields, even unused ones, and then explicitly initialising them, makes the intent clearer.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

El Tangas

Level: Raving Lunatic

Joined: Thu. Jul 14, 2016

Posts: 2762 View posts

#50

Posted by El Tangas: Thu. Sep 14, 2017 - 04:55 PM

alank2 wrote:

I suppose I could move:
        BrightTimer=0;
        ThisBright=((Dimmed)?0:Configuration.LampboardBright);

        Row++;
to where each delay is. I just hate to repeat code like that, but perhaps it is for the best. Can you do a function within a function for something like this?

Nested functions exist only as a gcc extension, they are not standard: https://gcc.gnu.org/onlinedocs/g...

If you use C++, you have lambda functions which are a kind of nested functions: https://stackoverflow.com/questi...

Otherwise, has previously mentioned, you would have to use function-like macros.

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11398 View posts

Location: Location, Location

#51

Posted by joeymorin: Thu. Sep 14, 2017 - 05:06 PM

There's also a lot of duplication of expressions in your posted code. Lots of ways to reduce that, making it clearer and easier to maintain/debug.

For example:

        // columns input with pullup
        LBKBSHARED_PORT      |=    (1 << LBKBSHARED_COL0_PIN) |
                                   (1 << LBKBSHARED_COL1_PIN) |
                                   (1 << LBKBSHARED_COL2_PIN) |
                                   (1 << LBKBSHARED_COL3_PIN) |
                                   (1 << LBKBSHARED_COL4_PIN);
        DDR(LBKBSHARED_PORT) &= ~( (1 << LBKBSHARED_COL0_PIN) |
                                   (1 << LBKBSHARED_COL1_PIN) |
                                   (1 << LBKBSHARED_COL2_PIN) |
                                   (1 << LBKBSHARED_COL3_PIN) |
                                   (1 << LBKBSHARED_COL4_PIN) );

That can be simpler:

#define LBKBSHARED_COL_ALL_MASK ((1 << LBKBSHARED_COL0_PIN) | \
                                 (1 << LBKBSHARED_COL1_PIN) | \
                                 (1 << LBKBSHARED_COL2_PIN) | \
                                 (1 << LBKBSHARED_COL3_PIN) | \
                                 (1 << LBKBSHARED_COL4_PIN))
.
.
.
        // columns input with pullup
        LBKBSHARED_PORT      |=  LBKBSHARED_COL_ALL_MASK;
        DDR(LBKBSHARED_PORT) &= ~LBKBSHARED_COL_ALL_MASK;

And the same mask is used multiple times later, in cases 0, 2, 3, and 5:

            case 0:
              //row 0
              DDR(KEYBOARD_PORT)     |= (1 << KEYBOARD_ROW0_PIN);
              __builtin_avr_delay_cycles(SETTLE_DELAY);
              ((uint8_t*)&AInput)[0]  = (~PIN(LBKBSHARED_PORT) &
                                          LBKBSHARED_COL_ALL_MASK) >> 1;
              DDR(KEYBOARD_PORT)     &= ~(1 << KEYBOARD_ROW0_PIN);
              break;

And again:

        // columns output
        DDR(LBKBSHARED_PORT) |= LBKBSHARED_COL_ALL_MASK;

Many of these will 'fall away' once you move to bitfields, but there are plenty of other opportunities to simplify with the use of masks and other macros.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

AVR Freaks