Jump to content
Sign in to follow this  
derekm

I2c_driver.h Modifications - Hardware Ack

Recommended Posts

derekm    0

Hi everone!

 

I'm new to PIC (or even micro) programming and Sourceboost, and I'm having a lot of fun. I've been messing around with i2c for about a week, and everything is going well.

 

I'm a little confused about why i2c_write doesn't return the ACKSTAT if the write was successful in HW mode, but it does in software mode.

 

Or better yet, instead of:

 

return (0); // successful exit

 

something like:

 

return (l_ackstat ? 2 : 0); /* 2 == no ack, 0 == success */

 

(with l_ackstat defined like the other l_* bits)

 

So we can detect a no-ack condition right within the i2c driver, instead of our "userland" code - which would have to depend on whether the chip is in HW, or SW mode.

 

I've checked the forums, and another poster had mentioned a similar fix, but I think I'd rather see the i2c_write function deal with this, as that would follow the Principle of Least Astonishment.

 

The template part of the API would need to change to add this extra bit, but the i2c_driver.h is incomplete without it. Heck, we could do a i2c_driver2.h, and insert a deprecated warning in i2c_driver.h so as not to affect existing code, if API changes are a problem.

 

Obviously the ack status is important, especially when using the ack-polling method.

 

Is there a mechanism for submitting patches to the "official" source tree? It's a quick fix, and I'd be glad to do the patches. (In addition, there doesn't seem to be any collision checking with the SW portion either).

 

I know I can make the changes to my local copy, but there is something to be said for support in future versions.

 

Any input on my suggestions?

 

Thanks!

Share this post


Link to post
Share on other sites
derekm    0
The template part of the API would need to change to add this extra bit, but the i2c_driver.h is incomplete without it. Heck, we could do a i2c_driver2.h, and insert a deprecated warning in i2c_driver.h so as not to affect existing code, if API changes are a problem.

 

I did the modifications, and actually the API didn't need to change that much. None of the template stuff needed changing, but I did change the return codes from the i2c_write function so that the return codes are different for the different exit reasons (i.e. 1 = collision, 2 = no ack).

 

Once again this is useful for the ack polling stuff.

 

So going back to the original question, what are the odds of getting some of this stuff committed somewhere other than my local repository?

Share this post


Link to post
Share on other sites
Dave    0

derekm,

So going back to the original question, what are the odds of getting some of this stuff committed somewhere other than my local repository?
The odds are very good if we beleive that the additional functionality is worth while. Please supply the modified sources and details of the changes made so they can be reviewed. Send them to support@sourceboost.com

 

Regards

Dave

Share this post


Link to post
Share on other sites
derekm    0
The odds are very good if we beleive that the additional functionality is worth while. Please supply the modified sources and details of the changes made so they can be reviewed. Send them to support@sourceboost.com

 

Thanks for the quick response. I've sent in the modifications.

 

(edit: added code snippet):

 

*** i2c_driver.h.orig	Tue Jan 29 21:35:04 2008
--- i2c_driver.h	Thu Jun 19 14:07:35 2008
***************
*** 440,445 ****
--- 440,446 ----
  volatile bit l_rw@T_i2c_SSPSTAT.i2c_RW,l_wcol@T_i2c_SSPCON1.i2c_WCOL;
  volatile bit l_rcen@T_i2c_SSPCON2.i2c_RCEN, l_pen@T_i2c_SSPCON2.i2c_PEN, l_sen@T_i2c_SSPCON2.i2c_SEN;
  volatile bit l_rsen@T_i2c_SSPCON2.i2c_RSEN, l_acken@T_i2c_SSPCON2.i2c_ACKEN;
+ 	volatile bit l_ackstat@T_i2c_SSPCON2.i2c_ACKSTAT;

  char BitMask;
  bit local_ack;
***************
*** 465,471 ****
	  while (l_acken || l_rcen || l_pen || l_rsen || l_sen || l_rw || !l_sspif)	
		  if (T_MODE & i2c_reset_wdt)
			  clear_wdt();
! 								
	  return (0); // successful exit
  }

--- 466,475 ----
	  while (l_acken || l_rcen || l_pen || l_rsen || l_sen || l_rw || !l_sspif)	
		  if (T_MODE & i2c_reset_wdt)
			  clear_wdt();
! 				
! 		if (l_ackstat)
! 			return (2); // no-ack error exit	
! 									
	  return (0); // successful exit
  }

***************
*** 515,527 ****
  delay_us(dly);

  // get the status bit
! 	local_ack = l_sda;
  delay_us(dly);

  l_scl_tris = 0; // drive SCL low	
  l_acken = 0;
  delay_us(dly);
! 	return(local_ack);
 }


--- 519,535 ----
  delay_us(dly);

  // get the status bit
! 	local_ack = l_sda; 
  delay_us(dly);

  l_scl_tris = 0; // drive SCL low	
  l_acken = 0;
  delay_us(dly);
! 	
! 	if (local_ack) 
! 		return (2); // no-ack error exit
! 	
! 	return (0);
 }

Edited by derekm

Share this post


Link to post
Share on other sites

Your content will need to be approved by a moderator

Guest
You are commenting as a guest. If you have an account, please sign in.
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoticons maximum are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
Sign in to follow this  

×