 |
 |
| View previous topic :: View next topic |
| Author |
Message |
elcrcp
Joined: 11 Mar 2016
Posts: 62
Location: izmir / Turkey
|
| Why I2C isr stops working of a 'for' loop? |
 Posted: Tue Mar 15, 2016 4:05 pm |
 |
|
Hello again,
I'm still developing the code I asked for help earlier. Things are working great to a point but now I'm stuck with another phenomenon  what I'm trying to do is to create a I2C network with 1 master and 4-5 slaves. but I started with 1 master and slave. I stucked with I2C earlier but now it's done, I can write to slave, I can read from slave and so I can send commands via I2C. My slave is working due to command bytes coming from I2C (controlling a bipolar step motor). More precisely, I send data from I2C to define which motor to control, which direction, how many steps etc. and I can send and receive these datas perfectly.
Before I send control data to slave, I read from slave to check if motor is busy or avaible, if motor is avaible then I send control data from master to slave. everything until here is perfect, now the problem is;
if I send data when motor is avaible, it starts to run as I wanted, and if I wait until motor is avaible again and re send a data, it is working again with no problem. But if I try to send data while motor is busy, I get busy message from slave (as it should be) and master does not attempt to send data to slave (this is also ok) but slave is stopping running "for" loop. I think it does not return into 'for' loop after I2C interrupt.
This is master code, on ext interrupt, asking slave for motor status and sends data or stops due to that status
| Code: | //-------------Master--------------------------
#include <16F1827.H>
#fuses NOWDT,NOPUT,BROWNOUT,NOLVP, MCLR, NOCPD, NOBROWNOUT, NOCLKOUT
#FUSES INTRC_IO, NOIESO, NOFCMEN, NOWRT, PLL_SW, NOSTVREN
#use delay(internal = 8000000)
#use i2c(MASTER, SDA=PIN_B1, SCL=PIN_B4)
#use rs232(baud=9600,parity=N,xmit=PIN_A0,rcv=PIN_A1,bits=8)
#byte TRISA = 0x08C
#byte TRISB = 0x08D
#byte OSCCON = 0x099
#byte OSCTUNE = 0x098
#byte DACCON0 = 0x118
#byte SRCON0 = 0x11A
#byte APFCON0 = 0x11D //ALTERNATE PIN FUNCTION CONTROL REGISTER 0
#byte APFCON1 = 0x11E //ALTERNATE PIN FUNCTION CONTROL REGISTER 1
//#define SLAVE1_WRT_ADDR 0xA8
//#define SLAVE1_READ_ADDR 0xA9
//====================================
const int8 slv_wrt_addr=0xA8, slv_rd_addr=0xA9;
volatile int8 motor_status, check_status;
void send_data_i2c(void);
void get_data_i2c(void);
#INT_EXT
void ext_isr(void)
{
get_data_i2c();
printf("Motor Status : %X\n\r",motor_status);
if(motor_status==0x07)
{
send_data_i2c();
printf("Data Sent\n\r");
}
else if(motor_status==0x70)
{
printf("Motor is busy, can't send data\n\r");
}
}
void main()
{
check_status=0;
TRISA = 0x20;
TRISB = 0x13;
OSCCON = 0x70;
OSCTUNE = 0;
DACCON0 = 0;
SRCON0 = 0;
setup_adc_ports(NO_ANALOGS);
enable_interrupts(INT_EXT);
enable_interrupts(GLOBAL);
while(1)
{
}
}
void send_data_i2c()
{
i2c_start();
i2c_write(slv_wrt_addr);
i2c_write(0x0F);//data array start sign
delay_ms(10);
i2c_write(0x10);//Control Data, 6.bit Motor Select, 4. Bit Motor Direction Select, 2. Bit EOW Hold-Release
delay_ms(10); // 0 to m1, 1 to m2 - 0 to CCW, 1 to CW - 0 to Release, 1 to Hold
i2c_write(0x00);// motor step amount low byte
delay_ms(10);
i2c_write(0x10);//motor step amount high byte
delay_ms(10);
i2c_write(0xF0);//data array stop sign
delay_ms(10);
i2c_stop();
delay_ms(100);
output_toggle(pin_b7);
}
void get_data_i2c()
{
i2c_start();
i2c_write(slv_rd_addr);
motor_status=i2c_read(0);//get motor status
i2c_stop();
delay_ms(10);
}
|
These are related parts of slave code, there is a pile of code with repeated actions, so I'm not sending all of program to keep it short
This part is declarations etc...
| Code: | ///------------------Slave-------------------------
#include <16F1827.H>
#fuses NOWDT,NOPUT,NOLVP,MCLR,NOCPD,NOBROWNOUT,NOCLKOUT
#FUSES INTRC_IO,NOIESO,NOFCMEN,NOWRT,PLL_SW,NOSTVREN
#use delay(internal = 8000000)
#use i2c(SLAVE, SDA=PIN_B1, SCL=PIN_B4, ADDRESS=0xA8)
#use rs232(baud=9600,parity=N,xmit=PIN_B7,rcv=PIN_B6,bits=8)
#byte TRISA = 0x08C
#byte TRISB = 0x08D
#byte OSCCON = 0x099
#byte OSCTUNE = 0x098
#byte DACCON0 = 0x118
#byte SRCON0 = 0x11A
#byte APFCON0 = 0x11D //ALTERNATE PIN FUNCTION CONTROL REGISTER 0
#byte APFCON1 = 0x11E //ALTERNATE PIN FUNCTION CONTROL REGISTER 1
volatile int8 datain[8];
volatile int8 state, readcnt, m2_current_step, m1_current_step, motor_status;
volatile unsigned long stepamount, temp_step; //16 bit motor step amount
volatile int1 data_ready, motor_direction, hold_release, motor_select; // 1 hold 0 relase - 1 CW 0 CCW - 1 data ready 0 not ready - 0 to m1 1 to m2 |
This is interrupt routine part, getting an array of data or sending status data
| Code: | #INT_SSP
void ssp_isr(void)
{
state = i2c_isr_state();
if(state < 0x80) // Master is sending data
{
datain[readcnt] = i2c_read();
readcnt=readcnt+1;
if(readcnt==6){
data_ready=1;
readcnt=0;
}
}
if(state >= 0x80) // Master is requesting data from slave
{
i2c_write(motor_status);
}
} |
This is main routine, if an array of correct sequenced data received, getting releated values from that array. then changing motor_status variable to 0x70 which means motor is busy, then calling motor run function, after finishing this function, changing motor_status to 0x07 which means avaible
| Code: | void main ()
{
readcnt=0;
motor_status=0x07;
data_ready=0;
m1_current_step=0;
m2_current_step=0;
i2c_SlaveAddr(0xA8);
TRISA = 0x20;
TRISB = 0xD3;
OSCCON = 0x70;
OSCTUNE = 0;
DACCON0 = 0;
SRCON0 = 0;
setup_adc_ports(NO_ANALOGS);//
enable_interrupts(INT_SSP);
enable_interrupts(INT_EXT);
enable_interrupts(GLOBAL);
while(1)
{
if ((data_ready==1)&&(datain[1]=0x0F)&&(datain[5]=0xF0))
{
motor_select=bit_test(datain[2],6);
motor_direction=bit_test(datain[2],4);
hold_release=bit_test(datain[2],2);
stepamount=make16(datain[4],datain[3]);
if(motor_direction==1)
printf("Direction: CW");
else if(motor_direction==0)
printf("Direction: CCW");
printf(" Step Amount: %Lu",stepamount);
if(hold_release==1)
printf(" End Condition : HOLD\n\r");
else if(hold_release==0)
printf(" End Condition : RELEASE\n\r");
for (int i=0;i<=6;++i)
{
datain[i]=0;
}
data_ready=0;
if(motor_select==0)
{
motor_status=0x70;
m1_run();
motor_status=0x07;
}
else if(motor_select==1)
{
motor_status=0x70;
m2_run();
motor_status=0x07;
}
}
}
}
//------------------------------- |
This function decides direction and calls releated function
| Code: | void m1_run()
{
if(motor_direction==0){
m1_move_ccw();}
else if(motor_direction==1){
m1_move_cw();}
} |
Now, when motor is busy, program is actually working in this 'for' loop, if I wait to until for is done and send another data from master, then there is no problem. But if I send data from master while program is in this for loop, master takes busy message but program does not return in this for loop , motor_status stays busy after that.
| Code: | void m1_move_cw()
{
output_bit(pin_b3,1);
for(temp_step=1;temp_step<=stepamount;++temp_step)
{
m1_current_step++;
if(m1_current_step==5){
m1_current_step=1;}
switch(m1_current_step)
{
case 1:
m1_step_1();
break;
case 2:
m1_step_2();
break;
case 3:
m1_step_3();
break;
case 4:
m1_step_4();
break;
}
delay_ms(2);
}
if(hold_release==0)
{
m1_motor_stop();
delay_ms(2);
output_bit(pin_b3,0);
}
}
//-------------------------------------- |
_________________
There is nothing you can't do if you try |
|
 |
temtronic
Joined: 01 Jul 2010
Posts: 9163
Location: Greensville,Ontario
|
|
| Posted: Tue Mar 15, 2016 4:53 pm |
|
|
3 things...
1) NEVER use printf() inside any ISR
2) NEVER use delay_ms() inside any ISR
3) ALWAYS add 'errors' to use rs232(...options...)
Jay |
|
|
elcrcp
Joined: 11 Mar 2016
Posts: 62
Location: izmir / Turkey
|
|
| Posted: Tue Mar 15, 2016 5:47 pm |
|
|
Thank you for advice, I didn't have any delay or printf in slave code but I added errors to rs232, changed master code as you suggested but that didn't changed the result so far. Still breaking 'for' loop when i2c interrupt occurs.
| Code: | #INT_EXT
void ext_isr(void)
{
button_signal=1;
}
void main()
{
...
...
...
while(1)
{
if(button_signal==1)
{
get_data_i2c();
printf("Motor Status : %X\n\r",motor_status);
if(motor_status==0x07)
{
send_data_i2c();
printf("Data Sent\n\r");
}
else if(motor_status==0x70)
{
printf("Motor is busy, can't send data\n\r");
}
button_signal=0;
}
}
}
|
_________________
There is nothing you can't do if you try |
|
|
elcrcp
Joined: 11 Mar 2016
Posts: 62
Location: izmir / Turkey
|
| case solved... |
| Posted: Tue Mar 15, 2016 6:32 pm |
|
|
problem is solved by changing
| Code: | if(state >= 0x80) // Master is requesting data from slave
{
i2c_write(motor_status);
}
} |
to
| Code: | if(state == 0x80) // Master is requesting data from slave
{
i2c_write(motor_status);
}
} |
in slave ssp isr code, I read manual for i2c_isr_state() but couldn't understand why this change solved my problem at all. It looks like program was stucking in ISR and could not return to where it left. Can anyone explain me this phenomenon please?
_________________
There is nothing you can't do if you try |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Wed Mar 16, 2016 2:46 am |
|
|
Fundamental problem.
You need to understand state 0x80, and that the current code is also wrong (it just doesn't hang the chip.....)
The key problem is in your _master_ code.
The sequence in I2C, is:
First byte = chip address and direction control.
On a 'write', second byte is the slave register address.
Then the data. The last byte of transmitted data is then NACKED, rather than ACKED.
On a read, the address set by the write is used as the starting address, followed by sequence of data transmissions. So normally a read is done by sending a write (to set the register address), followed by a 'restart' to turn the bus round.
You are trying to short-cut I2C, by not sending the register address. The actual behaviour of this depends on the PIC, so best to 'avoid' if you want code to work in the future. It can be made to work (as you have found), but can also lead to the I2C getting into a 'hung' state, and different chip families (even members of the same family) are all different when this is done....
Now on the I2C slave, the restart is usually used as the 'flag' to say 'load the data ready for the next transmission', and the PIC address sent for this both has to be read, and then the next byte loaded. But you are not actually having a restart. Hence the hardware gets into a twist.....
It's fluke that it actually works, but better to get the sequencing right:
(you seem to only send 3 bytes really, so setup to allow four bytes).
| Code: |
//master
//processor setups etc., omitted
int8 tx_data_array[4];
int8 rx_data_array[4];
//send 'num_bytes' of data from array at 'data', starting at 'reg_address'
void send_i2c(int8 * data, int8 reg_address, int8 num_bytes)
{
int8 ctr;
i2c_start();
i2c_write(slv_write_addr);
i2c_write(reg_address); //register address in the slave
for (ctr=0;ctr<num_bytes;ctr++)
i2c_write(data[ctr]);
i2c_stop();
}
//similar to read
void read_i2c(int8* data, int8 reg_address, int8 num_bytes)
{
int8 ctr;
i2c_start();
i2c_write(slv_write_addr);
i2c_write(reg_address); //register address in the slave
//at this point the register address has been setup in the slave
i2c_start(); //send a restart to turn the bus round
i2c_write(slv_rd_addr);
for (ctr=0;ctr<num_bytes;ctr++)
{
if (ctr==(num_bytes--1))
data[ctr]=i2c_read(0); //NACK last byte
else
data[ctr]=i2c_read();
}
i2c_stop();
}
//Now in the main code, to send
tx_data_array[0] = 0x10; //your control
tx_data_array[1] = 0; //step low byte
tx_data_array[2] = 0x10; //step high byte
send_i2c(tx_data_array, 0, 3); //send 3 bytes to slave
//or to read status
read_i2c(rx_data_array, 0, 1); //get one byte from I2C
//rx_data_array[0] will be your status
//Now in the slave
#define ARRAY_MAX 4
int8 datain[ARRAY_MAX];
int8 dataout[ARRAY_MAX];
//arrays for the data arriving and leaving.
#INT_SSP
void ssp_isr(void)
{
int8 state; //Generally keep variables local to your routines, unless
//they _need_ to be global
static int8 reg_address; //this needs to stay on successive calls - static
int8 dummy;
state = i2c_isr_state();
if (state==0)
dummy=i2c_read(); //read slave address byte - throw.
else if (state==1)
reg_address=i2c_read(); //read the transmitted register address
else if(state < 0x80) // Master is sending data
{
datain[reg_address++] = i2c_read();
//set your flag after the three bytes
if (reg_address==3)
data_ready=TRUE;
}
else if (state==0x80)
dummy=i2c_read(2); //read the turnaround byte, without releasing bus
if (state>=0x80)
{
//Now have to preload the data for the next read
i2c_write(dataout[reg_address++]);
}
if (reg_address==ARRAY_MAX)
reg_address--; //avoid array overflow.
}
//State 0x80, requires both a read _and_ a write. The higher states
//only require the write
|
Now I've fiddled quite a bit. The counter variable, and state is local to the routine. Always make variables 'local', unless the data is required to be distributed to another routine. It reduces the risk of variables 'accidentally' getting overwritten....
Have left out lots of declarations, so though I set 'data_ready' for example, I have not declared it.
You'd need to copy the motor status into dataout[0] for the code as shown.
You could just send the single byte, if this is all you need, but I decided to go a little more 'generic', and the read routine/transmission both handle a four byte array.
The three bytes you seem to actually need (control, and the two step bytes), will be in datain[0] to 2. Can't see why you have the two flag bytes, so have omitted these.
You don't need the huge delays between sending bytes on the I2C.
This is more generic than needed, since it allows larger arrays, and multi byte transmission/reception.
Probably some typos, I typed this in on the board here, so can easily have typed something wrong... |
|
|
elcrcp
Joined: 11 Mar 2016
Posts: 62
Location: izmir / Turkey
|
|
| Posted: Wed Mar 16, 2016 3:57 pm |
|
|
Thank you very much for broad explanation Ttelmah. Thats been a good education for me =) I'll try to implement your advices to my code
_________________
There is nothing you can't do if you try |
|
|
elcrcp
Joined: 11 Mar 2016
Posts: 62
Location: izmir / Turkey
|
|
| Posted: Thu Mar 17, 2016 4:40 pm |
|
|
Hello again Ttelmah, I read and totally understand both your comments and help context about i2c_isr_state() and I changed my code due to that. my slave ISR became:
| Code: | void ssp_isr(void)
{
int8 state, temp_var;
state = i2c_isr_state();
if(state <= 0x80) // Master is sending data
{
if(state==0x80)
{
temp_var=i2c_read(2); //as you said, 0x80 is to read and hold the line
}
else if(state==0)
{
temp_var=i2c_read(); // address match, don't need this..
}
else
{
datain[readcnt] = i2c_read(); // state 1 is used for register address but I dont use it and so I can take it as data, I think there is no problem with that
readcnt=readcnt+1;
if(readcnt==5)
{
data_ready=1;
readcnt=0;
}
}
}
if(state >= 0x80) // Master is requesting data from slave
{
i2c_write(motor_status);
}
} |
Now, I know this is how should it be but when I use this ISR, again I faced with previous problem. I2C is working, slave is responding anytime master tries to read but when master tries to read from slave, interrupt occurs and responds but program does not return to "for" loop it was working before interrupt. I tried a lot, tried blind shot changes on code, changed master I2C codes too but I got no results. And the top of it, below codes are working strangely fine although they should cause problems since they defy i2c_isr_status()
| Code: | void ssp_isr(void)
{
state = i2c_isr_state();
if(state < 0x80) // Master is sending data
{
datain[readcnt] = i2c_read();
readcnt=readcnt+1;
if(readcnt==6){
data_ready=1;
readcnt=0;
}
}
if(state == 0x80) // Master is requesting data from slave
{
i2c_write(motor_status);
}
} |
_________________
There is nothing you can't do if you try |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Fri Mar 18, 2016 3:14 am |
|
|
I don't think you have ever said 'what compiler version'?.
When the I2c_read(2) option was added, there were a number of versions
that did not release the bus on the subsequent write....
Problem as it stands, is that the bus can get 'out of sync'. If part of a transaction is corrupted/missed, it won't recover and start putting the data back where it should (readcnt will be wrong).
So:
| Code: |
#bit CKP=getenv("BIT:CKP")
void ssp_isr(void)
{
int8 state, temp_var;
state = i2c_isr_state();
if(state <= 0x80) // Master is sending data
{
if(state==0x80)
{
temp_var=i2c_read(2); //as you said, 0x80 is to read and hold the line
}
else if(state==0)
{
temp_var=i2c_read(); // address match, don't need this..
}
else
{
datain[state-1] = i2c_read();
if(state==5)
{
data_ready=1;
}
}
}
//This way the location in the array will always be synchronised to the
//start -> state sets to 0 at the start.
if(state >= 0x80) // Master is requesting data from slave
{
i2c_write(motor_status);
CKP=TRUE; //ensure clock releases
}
}
|
This ensures the clock releases (which I suspect is your problem). and
that the location of the data is automatically set by 'where' we are in the
transaction. 
How many bytes are you actually sending?. Your counter seems to imply 4?. |
|
|
|
|
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
