 |
 |
| View previous topic :: View next topic |
| Author |
Message |
gjs_rsdi
Joined: 06 Feb 2006
Posts: 468
Location: Bali
|
| C question (switch or .....) |
 Posted: Thu Sep 01, 2016 1:52 pm |
 |
|
Using 5 pins of PIC18F26K22 and two 74HC251 to read 16 switches
I realized late that if I will use the PIC18F46K22 I can read them directly and will design a new board, smaller, but in the mean time for my project to go forward I would like to use the already made boards with PIC18F26K22.
I can read the 16 switches with a "switch" (example below for 3).
My question is if this work can be done in a more efficient and elegant way in C?
| Code: | switch(digitaldatacnt)
{
case 0://0000
{
output_low(diginsltA);
output_low(diginsltB);
output_low(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
digdata0=1;
}
else
{
digdata0=0;
}
digitaldatacnt++;
}
break;
case 1://0001
{
output_high(diginsltA);
output_low(diginsltB);
output_low(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
digdata1=1;
}
else
{
digdata1=0;
}
digitaldatacnt++;
}
break;
case 2://0010
{
output_low(diginsltA);
output_high(diginsltB);
output_low(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
digdata2=1;
}
else
{
digdata2=0;
}
digitaldatacnt=0;
}
break;
} |
Any advice will be appreciated.
Best wishes
Joe |
|
 |
temtronic
Joined: 01 Jul 2010
Posts: 9225
Location: Greensville,Ontario
|
|
| Posted: Thu Sep 01, 2016 2:30 pm |
|
|
Are you 'committed' to using the 74HC251s ?
If not, have a look at the MCP23016. It's a 16 bit I/O 'expander' that could do the job, perhaps easier, is more 'flexible', less PCB area too ??
Only needs 2 I2C pins, so you 'save ' 3 PIC pins !
options, always think of options...
Jay |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19506
|
|
| Posted: Thu Sep 01, 2016 2:36 pm |
|
|
Think for a moment:
| Code: |
//just for the eight on the first multiplexer:
output_bit(DIGinsltA,bit_test(digitaldatacnt,0));
output_bit(DIGinsltB,bit_test(digitaldatacnt,1));
output_bit(DIGinsltC,bit_test(digitaldatacnt,2));
|
Would work for all the first eight counts.
Then if your 'digdata' bits were declared as a union between a byte and an array of single bits, you could use:
| Code: |
bit_set(digdata.byte,digitaldatacnt);
|
(or bit_clear if the input is not active).
So the whole thing could be done with only about ten lines of code. |
|
|
gjs_rsdi
Joined: 06 Feb 2006
Posts: 468
Location: Bali
|
|
| Posted: Thu Sep 01, 2016 4:38 pm |
|
|
Thank you for the answers temtronic and Ttelmah
I will use the PIC18F46K22 on the new board so no need for any mux/IO expander.
Ttelmah, can you explain in more details regarding:
| Code: | | bit_set(digdata.byte,digitaldatacnt); |
I know how to put the bits together in one byte but I don't understand how I can output them to give me
0000 to 1111
Best wishes
Joe |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9225
Location: Greensville,Ontario
|
|
| Posted: Thu Sep 01, 2016 5:48 pm |
|
|
Great ! another convert to using my favorite PIC !!!
Lots of I/O, mem and works full speed from 3 to 5 volts !!
jay |
|
|
gjs_rsdi
Joined: 06 Feb 2006
Posts: 468
Location: Bali
|
|
| Posted: Thu Sep 01, 2016 7:09 pm |
|
|
Yes Jay 
I moved from 2520/4520 to 26K22/46K22 even that most cases until now using between 12% to 38% of they memory
They can do about everything one needs except making coffee and scratch your back 
Also for 18 pins using 16F1847, just excellent!
No need to learn so many hardware's
My problem is that I am not so good in C, my programs in C are more like simplified assembler 
Do my best to learn!
Best wishes
Joe |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19506
|
|
| Posted: Fri Sep 02, 2016 1:01 am |
|
|
The point about the 'bit set', was I'm assuming you have a set of outputs like:
| Code: |
int1 digdata0, digdata1, digdata2, digdata3, digdata4, digdata5, digdata6, digdata7;
|
Why not an array?.
If you declared as:
| Code: |
union {
int8 dbyte;
int1 data[8];
} dig;
|
Then if you were looping just using the loop counter as the count to feed to the multiplexer, you could either use:
dig.data[digitaldatacnt] = input(diginDATA);
(I think the neat way of doing it...)
or with the separate bits, you could use #byte to locate a byte to point to the same location in memory, and use:
bit_set(byte_in_memory,digitaldatacnt);
instead of
digdataxx=1;
and obviously bit clear for =0, where 'byte_in_memory', is placed where these variables are stored. |
|
|
gaugeguy
Joined: 05 Apr 2011
Posts: 303
|
|
| Posted: Fri Sep 02, 2016 6:36 am |
|
|
I have used the 74HC251s in a similar way before. If you are going to read all of the inputs at one time the most efficient way would be to order it in gray code scale order. Only one bit changes at a time.
000 starting
001 set one output
011 set one output
010 clear one output
110 set one output
111 set one output
101 clear one output
100 clear one output
...
Each time you change only one output bit and then read the input bits. There is no loop, everything is done in-line and goes very fast. |
|
|
gjs_rsdi
Joined: 06 Feb 2006
Posts: 468
Location: Bali
|
|
| Posted: Fri Sep 02, 2016 9:46 pm |
|
|
Sorry annoying, I just don't know how to use the advice's
I am posting the working program I wrote, tested in MPLAB v8.92 and
tested with the hardware also:
CCS PCH C Compiler, Version 5.059
| Code: | ///////////////////////////////////////////////////////////////////
//test program for reading 16 digital inputs///////////////////////
///////////////////////////////////////////////////////////////////
#include <18F26K22.h>
#device ADC=10
#FUSES NOWDT //No Watch Dog Timer
#FUSES INTRC_IO //
#FUSES PLLEN
#FUSES NOFCMEN //Fail-safe clock monitor disabled
#FUSES NOIESO //Internal External Switch Over mode disabled
#FUSES PUT //Power Up Timer
#FUSES BROWNOUT
#FUSES BORV29 //Brownout reset at 2.85V
#FUSES NOPBADEN //PORTB pins are configured as digital I/O on RESET
#FUSES NOHFOFST //High Frequency INTRC waits until stable before clocking CPU
#FUSES NOLVP //No low voltage prgming, B3(PIC16) or B5(PIC18) used for I/O
#FUSES NOXINST //Extended set extension and Indexed Addressing mode disabled (Legacy mode)
#use delay(internal=64MHz)
#use rs232(baud=9600, UART1, stream=PORT1, errors)
#ZERO_RAM
#define diginsltA PIN_B0
#define diginsltB PIN_B1
#define diginsltC PIN_B2
#define diginsltOE PIN_B3//active L for data0; H for data1 via inverter
#define diginDATA PIN_B4
#define flashled PIN_C3
#define txflashled PIN_A4
int flashledcnt=0;
int digitaldatacnt=0;
int data00,data01,data02,data03,data04,data05,data06,data07;
int data08,data09,data10,data11,data12,data13,data14,data15;
int tx1words=0;
int tx1dig0=0;
int tx1dig1=0;
int tx1wch=0;
int tx1timecnat=0;
int rxdata=0;
#include "T22DG.h"
///////////////////////////////////////////////////////////////////
#INT_TIMER3
void TIMER3_isr(void)
{
flashledcnt++;
tx1timecnat++;
if(flashledcnt==16)
{
output_toggle(flashled);//every 524ms
flashledcnt=0;
}
if(tx1timecnat==64)
{
enable_interrupts(INT_TBE);//every 2.096 seconds
output_toggle(txflashled);
tx1timecnat=0;
}
}
///////////////////////////////////////////////////////////////////
#INT_TBE
void TBE_isr(void)
{
switch(tx1words)
{
case 0:
{
fputc(85,PORT1);
tx1words++;
}
break;
case 1:
{
fputc(170,PORT1);
tx1wch=255;
tx1words++;
}
break;
case 2:
{
fputc(tx1dig0,PORT1);
tx1wch=tx1wch+tx1dig0;
tx1words++;
}
break;
case 3:
{
fputc(tx1dig1,PORT1);
tx1wch=tx1wch+tx1dig1;
tx1words++;
}
break;
case 4:
{
fputc(tx1wch,PORT1);
tx1words=0;
disable_interrupts(INT_TBE);
}
break;
}
}
///////////////////////////////////////////////////////////////////
/*
#int_RDA
void RDA_isr(void)
{
rxdata=getc();
}
*/
///////////////////////////////////////////////////////////////////
void main()
{
port_b_pullups(0xFF);
setup_timer_3(T3_INTERNAL | T3_DIV_BY_8);//32.75 ms overflow
enable_interrupts(INT_TIMER3);
disable_interrupts(INT_RDA);
disable_interrupts(INT_TBE);
enable_interrupts(GLOBAL);
while(TRUE)
{
delay_us(1);
DG();
}
}
|
The T22DG.h
| Code: | ///////////////////////////////////////////////////////////////////
//DIGITAL INPUTS READING///////////////////////////////////////////
///////////////////////////////////////////////////////////////////
void DG(void)
{
switch(digitaldatacnt)
{
case 0://0000
{
output_low(diginsltA);
output_low(diginsltB);
output_low(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data00=1;
tx1dig0=1;
}
else
{
data00=0;
tx1dig0=0;
}
digitaldatacnt++;
}
break;
case 1://0001
{
output_high(diginsltA);
output_low(diginsltB);
output_low(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data01=1;
tx1dig0=tx1dig0+2;
}
else
{
data01=0;
}
digitaldatacnt++;
}
break;
case 2://0010
{
output_low(diginsltA);
output_high(diginsltB);
output_low(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data02=1;
tx1dig0=tx1dig0+4;
}
else
{
data02=0;
}
digitaldatacnt++;
}
break;
case 3://0011
{
output_high(diginsltA);
output_high(diginsltB);
output_low(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data03=1;
tx1dig0=tx1dig0+8;
}
else
{
data03=0;
}
digitaldatacnt++;
}
break;
case 4://0100
{
output_low(diginsltA);
output_low(diginsltB);
output_high(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data04=1;
tx1dig0=tx1dig0+16;
}
else
{
data04=0;
}
digitaldatacnt++;
}
break;
case 5://0101
{
output_high(diginsltA);
output_low(diginsltB);
output_high(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data05=1;
tx1dig0=tx1dig0+32;
}
else
{
data05=0;
}
digitaldatacnt++;
}
break;
case 6://0110
{
output_low(diginsltA);
output_high(diginsltB);
output_high(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data06=1;
tx1dig0=tx1dig0+64;
}
else
{
data06=0;
}
digitaldatacnt++;
}
break;
case 7://0111
{
output_high(diginsltA);
output_high(diginsltB);
output_high(diginsltC);
output_low(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data07=1;
tx1dig0=tx1dig0+128;
}
else
{
data07=0;
}
digitaldatacnt++;
}
break;
case 8://1000
{
output_low(diginsltA);
output_low(diginsltB);
output_low(diginsltC);
output_high(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data08=1;
tx1dig1=1;
}
else
{
data08=0;
tx1dig1=0;
}
digitaldatacnt++;
}
break;
case 9://1001
{
output_high(diginsltA);
output_low(diginsltB);
output_low(diginsltC);
output_high(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data09=1;
tx1dig1=tx1dig1+2;
}
else
{
data09=0;
}
digitaldatacnt++;
}
break;
case 10://1010
{
output_low(diginsltA);
output_high(diginsltB);
output_low(diginsltC);
output_high(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data10=1;
tx1dig1=tx1dig1+4;
}
else
{
data10=0;
}
digitaldatacnt++;
}
break;
case 11://1011
{
output_high(diginsltA);
output_high(diginsltB);
output_low(diginsltC);
output_high(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data11=1;
tx1dig1=tx1dig1+8;
}
else
{
data11=0;
}
digitaldatacnt++;
}
break;
case 12://1100
{
output_low(diginsltA);
output_low(diginsltB);
output_high(diginsltC);
output_high(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data12=1;
tx1dig1=tx1dig1+16;
}
else
{
data12=0;
}
digitaldatacnt++;
}
break;
case 13://1101
{
output_high(diginsltA);
output_low(diginsltB);
output_high(diginsltC);
output_high(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data13=1;
tx1dig1=tx1dig1+32;
}
else
{
data13=0;
}
digitaldatacnt++;
}
break;
case 14://1110
{
output_low(diginsltA);
output_high(diginsltB);
output_high(diginsltC);
output_high(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data14=1;
tx1dig1=tx1dig1+64;
}
else
{
data14=0;
}
digitaldatacnt++;
}
break;
case 15://1111
{
output_high(diginsltA);
output_high(diginsltB);
output_high(diginsltC);
output_high(diginsltOE);
delay_us(1);
if(input(diginDATA))
{
data15=1;
tx1dig1=tx1dig1+128;
}
else
{
data15=0;
}
digitaldatacnt=0;
}
break;
}
}
/////////////////////////////////////////////////////////////////// |
Any advice/correction will be much appreciated
Best wishes
Joe
Last edited by gjs_rsdi on Sat Sep 03, 2016 3:18 pm; edited 3 times in total |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Sat Sep 03, 2016 12:09 am |
|
|
| Quote: |
#int_RDA
void RDA_isr(void)
{
delay_us(1);
} |
This will never work. You should either remove the above routine or put
an fgetc() statement in it to read the character in the RDA buffer and clear the interrupt. |
|
|
gjs_rsdi
Joined: 06 Feb 2006
Posts: 468
Location: Bali
|
|
| Posted: Sat Sep 03, 2016 12:22 am |
|
|
Thanks for the reply PCM programmer.
I am using just the tx, I was thinking enough to put just delay inside the rx.
Will correct the interrupt, the program itself working as int RDA is disabled.
Best wishes
Joe |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19506
|
|
| Posted: Sat Sep 03, 2016 1:12 am |
|
|
Have a look at this thread:
<http://www.ccsinfo.com/forum/viewtopic.php?t=55452>
This has been covered hundreds of times here. If you did enable the interrupt, it'd be a 'killer'... |
|
|
gjs_rsdi
Joined: 06 Feb 2006
Posts: 468
Location: Bali
|
|
| Posted: Sat Sep 03, 2016 1:41 am |
|
|
Hi Ttelmah
I changed the code as PCM programmer suggested
I understand that the code without reading the buffer will not clear the interrupt.
Tested with the hardware also:
| Code: | #int_RDA
void RDA_isr(void)
{
delay_us(1);
} |
If I am sending a message, the led's continue to flash, but the TX wont send any data
| Code: | #int_RDA
void RDA_isr(void)
{
rxdata=getc();
} |
Works perfect 
PCM programmer advice was very helpful to understand the point
Best wishes
Joe |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Sat Sep 03, 2016 7:07 am |
|
|
Your code has a lot of duplication which makes your program very long and difficult to maintain. Just a tiny change to one of the switch readings means you'll have to copy this code modification 16 times. The chance of forgetting a single copy is huge, resulting in difficult to find bugs.
Rule of thumb is that whenever you have to copy/paste the same code 3 or more times, then you have to remove this duplication. Possible techniques are to create a new sub-function or creating a macro.
| Code: | int data00=0;
int data01,data02,data03,data04,data05,data06,data07;
int data08,data09,data10,data11,data12,data13,data14,data15;
|
Here again, lots of duplication. Your code is easier to maintain with a construct like: | Code: | #define NUMBER_OF_INPUTS 16
int8 data[NUMBER_OF_INPUTS]; |
I know you already have working code now, but reducing complexity is a very good learning exercise that will make you a better programmer. |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Sat Sep 03, 2016 7:15 am |
|
|
while you say adopting PCMP's code prevents crashing
what you have makes no sense as:
since you never use rxdata in the code !!
AND when a new character comes in -if it does- you
have no way to build a string or do anything useful.
better to just not enable the RDA ISR if you are not using it for anything.... |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
