8000 Double I2C read in one transaction skips a clock pulse (#5528) (based on I2C reduce iRAM) by TD-er · Pull Request #6592 · esp8266/Arduino · GitHub
[go: up one dir, main page]
Skip to content

Double I2C read in one transaction skips a clock pulse (#5528) (based on I2C reduce iRAM) #6592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 27 commits into from
Closed

Double I2C read in one transaction skips a clock pulse (#5528) (based on I2C reduce iRAM) #6592

Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7126fbf
Reduce the IRAM (and heap) usage of I2C code
earlephilhower Jul 22, 2019
5ba2dd3
Make most variables ints, not uint8_ts
earlephilhower Jul 22, 2019
f55ab36
Make local flag vars int
earlephilhower Jul 22, 2019
37e10b9
Factor out !scl in onSdaChange
earlephilhower Jul 22, 2019
423a5bf
Make tiny twi_reply inline
earlephilhower Jul 22, 2019
62f9599
Inline additional twi_** helper functions
earlephilhower Jul 22, 2019
014f87f
Convert state machine to 1-hot for faster lookup
earlephilhower Jul 22, 2019
9ae9af1
Factor out twi_status setting
earlephilhower Jul 24, 2019
b975996
Use a struct to hold globals for TWI
earlephilhower Jul 24, 2019
52bf8ac
Use enums for states, move one more var to twi struct
earlephilhower Jul 25, 2019
31e54a0
Save 4 heap bytes by reprdering struct
8000 earlephilhower Jul 25, 2019
fd8b6c5
Convert to C++ class, clean up code
earlephilhower Jul 26, 2019
0cdca29
Run astyle core.conf, clean up space/tab/etc.
earlephilhower Jul 26, 2019
eb7b2b9
Merge branch 'master' into reducei2ciram
earlephilhower Aug 8, 2019
3552bfc
Merge branch 'master' into reducei2ciram
earlephilhower Aug 19, 2019
51e1e34
Merge branch 'master' into reducei2ciram
earlephilhower Sep 15, 2019
1c14e9e
Merge branch 'master' into reducei2ciram
d-a-v Oct 3, 2019
1c63358
Double I2C read in one transaction skips a clock pulse (#5528)
TD-er Oct 3, 2019
f04d65c
Merge branch 'master' into reducei2ciram
earlephilhower Oct 4, 2019
f67ddf5
Add enum use comment, rename twi::delay, fix SDA/SCL_READ bool usage
earlephilhower Oct 4, 2019
dd6a854
Replace clock stretch repeated code w/inline loop
earlephilhower Oct 4, 2019
a23864a
Remove slave code when not using slave mode
earlephilhower Oct 4, 2019
eb85c68
Double I2C read in one transaction skips a clock pulse (#5528)
TD-er Oct 3, 2019
900bf65
Merge remote-tracking branch 'origin/bugfix/issue5528_I2C_reduceiram'…
TD-er Oct 14, 2019
138e1ec
Restyle core_esp8266_si2c.cpp
TD-er Oct 14, 2019
0dac904
Merge branch 'master' into bugfix/issue5528_I2C_reduceiram
d-a-v Oct 14, 2019
a6c3700
Add libraries/Wire to restyle.sh
TD-er Oct 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Convert state machine to 1-hot for faster lookup 
GCC won't use a lookup table for the TWI state machine, so it ends up
using a series of straight line compare-jump, compare-jumps to figure
out which branch of code to execute for each state.  For branches that
have multiple states that call them, this can expand to a lot of code.

Short-circuit the whole thing by converting the FSM to a 1-hot encoding
while executing it, and then just and-ing the 1-hot state with the
bitmask of states with the same code.

Sketch uses 270719 bytes (25%) of program storage space. Maximum is 1044464 bytes.
Global variables use 27944 bytes (34%) of dynamic memory, leaving 53976 bytes for local variables. Maximum is 81920 bytes.

401000cc l     F .text1	00000014 twi_delay
401000f4  w    F .text1	0000003b twi_releaseBus
4010015c g     F .text1	00000246 twi_onTwipEvent
401003c0 l     F .text1	000001b1 onSdaChange
40100580 l     F .text1	000002da onSclChange
4010085c l     F .text1	0000003b onTimer

0x00000000401073cc                _text_end = ABSOLUTE (.)

Saves 228 bytes of IRAM vs. master, uses 32 additional bytes of heap.
  • Loading branch information
@earlephilhower
earlephilhower committed Jul 22, 2019
commit 014f87f83bbb89fa56d0dde8cb8dbd7c69aad67e
252 changes: 107 additions & 145 deletions cores/esp8266/core_esp8266_si2c.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,14 @@ static void eventTask(ETSEvent *e)
}
}

// The state machine is converted from a 0...15 state to a 1-hot encoded state, and then
// compared to the logical-or of all states with the same branch. This removes the need
// for a large series of straight-line compares. The biggest win is when multiple states
// all have the same branch (onSdaChange), but for others there is some benefit, still.
#define S2M(x) (1<<(x))
// Shorthand for if the state is any of the or'd bitmask x
#define IFSTATE(x) if (twip_state_mask & (x))

void ICACHE_RAM_ATTR onSclChange(void)
{
unsigned int sda;
Expand All @@ -532,151 +540,124 @@ void ICACHE_RAM_ATTR onSclChange(void)

twip_status = 0xF8; // reset TWI status

switch (twip_state)
{
case TWIP_IDLE:
case TWIP_WAIT_STOP:
case TWIP_BUS_ERR:
int twip_state_mask = S2M(twip_state);
IFSTATE(S2M(TWIP_START)|S2M(TWIP_REP_START)|S2M(TWIP_SLA_W)|S2M(TWIP_READ)) {
if (!scl) {
// ignore
break;
} else {
bitCount--;
twi_data <<= 1;
twi_data |= sda;

case TWIP_START:
case TWIP_REP_START:
case TWIP_SLA_W:
case TWIP_READ:
if (!scl) {
// ignore
if (bitCount != 0) {
// continue
} else {
bitCount--;
twi_data <<= 1;
twi_data |= sda;

if (bitCount != 0) {
// continue
twip_state = TWIP_SEND_ACK;
}
}
} else IFSTATE(S2M(TWIP_SEND_ACK)) {
if (scl) {
// ignore
} else {
if (twip_mode == TWIPM_IDLE) {
if ((twi_data & 0xFE) != twi_addr) {
// ignore
} else {
twip_state = TWIP_SEND_ACK;
SDA_LOW();
}
}
break;

case TWIP_SEND_ACK:
if (scl) {
// ignore
} else {
if (twip_mode == TWIPM_IDLE) {
if ((twi_data & 0xFE) != twi_addr) {
// ignore
} else {
SDA_LOW();
}
if (!twi_ack) {
// ignore
} else {
if (!twi_ack) {
// ignore
} else {
SDA_LOW();
}
SDA_LOW();
}
twip_state = TWIP_WAIT_ACK;
}
break;

case TWIP_WAIT_ACK:
if (scl) {
// ignore
} else {
if (twip_mode == TWIPM_IDLE) {
if ((twi_data & 0xFE) != twi_addr) {
SDA_HIGH();
twip_state = TWIP_WAIT_STOP;
} else {
SCL_LOW(); // clock stretching
SDA_HIGH();
twip_mode = TWIPM_ADDRESSED;
if (!(twi_data & 0x01)) {
twip_status = TW_SR_SLA_ACK;
twi_onTwipEvent(twip_status);
bitCount = 8;
twip_state = TWIP_SLA_W;
} else {
twip_status = TW_ST_SLA_ACK;
twi_onTwipEvent(twip_status);
twip_state = TWIP_SLA_R;
}
}
twip_state = TWIP_WAIT_ACK;
}
} else IFSTATE(S2M(TWIP_WAIT_ACK)) {
if (scl) {
// ignore
} else {
if (twip_mode == TWIPM_IDLE) {
if ((twi_data & 0xFE) != twi_addr) {
SDA_HIGH();
twip_state = TWIP_WAIT_STOP;
} else {
SCL_LOW(); // clock stretching
SDA_HIGH();
if (!twi_ack) {
twip_status = TW_SR_DATA_NACK;
twip_mode = TWIPM_ADDRESSED;
if (!(twi_data & 0x01)) {
twip_status = TW_SR_SLA_ACK;
twi_onTwipEvent(twip_status);
twip_mode = TWIPM_WAIT;
twip_state = TWIP_WAIT_STOP;
bitCount = 8;
twip_state = TWIP_SLA_W;
} else {
twip_status = TW_SR_DATA_ACK;
twip_status = TW_ST_SLA_ACK;
twi_onTwipEvent(twip_status);
bitCount = 8;
twip_state = TWIP_READ;
twip_state = TWIP_SLA_R;
}
}
}
break;

case TWIP_SLA_R:
case TWIP_WRITE:
if (scl) {
// ignore
} else {
bitCount--;
(twi_data & 0x80) ? SDA_HIGH() : SDA_LOW();
twi_data <<= 1;

if (bitCount != 0) {
// continue
SCL_LOW(); // clock stretching
SDA_HIGH();
if (!twi_ack) {
twip_status = TW_SR_DATA_NACK;
twi_onTwipEvent(twip_status);
twip_mode = TWIPM_WAIT;
twip_state = TWIP_WAIT_STOP;
} else {
twip_state = TWIP_REC_ACK;
twip_status = TW_SR_DATA_ACK;
twi_onTwipEvent(twip_status);
bitCount = 8;
twip_state = TWIP_READ;
}
}
break;

case TWIP_REC_ACK:
if (scl) {
// ignore
} else {
SDA_HIGH();
twip_state = TWIP_READ_ACK;
}
break;
}
} else IFSTATE(S2M(TWIP_SLA_R)|S2M(TWIP_WRITE)) {
if (scl) {
// ignore
} else {
bitCount--;
(twi_data & 0x80) ? SDA_HIGH() : SDA_LOW();
twi_data <<= 1;

case TWIP_READ_ACK:
if (!scl) {
// ignore
if (bitCount != 0) {
// continue
} else {
twi_ack_rec = !sda;
twip_state = TWIP_RWAIT_ACK;
twip_state = TWIP_REC_ACK;
}
break;

case TWIP_RWAIT_ACK:
if (scl) {
// ignore
}
} else IFSTATE(S2M(TWIP_REC_ACK)) {
if (scl) {
// ignore
} else {
SDA_HIGH();
twip_state = TWIP_READ_ACK;
}
} else IFSTATE(S2M(TWIP_READ_ACK)) {
if (!scl) {
// ignore
} else {
twi_ack_rec = !sda;
twip_state = TWIP_RWAIT_ACK;
}
} else IFSTATE(S2M(TWIP_RWAIT_ACK)) {
if (scl) {
// ignore
} else {
SCL_LOW(); // clock stretching
if (twi_ack && twi_ack_rec) {
twip_status = TW_ST_DATA_ACK;
twi_onTwipEvent(twip_status);
twip_state = TWIP_WRITE;
} else {
SCL_LOW(); // clock stretching
if (twi_ack && twi_ack_rec) {
twip_status = TW_ST_DATA_ACK;
twi_onTwipEvent(twip_status);
twip_state = TWIP_WRITE;
} else {
// we have no more data to send and/or the master doesn't want anymore
twip_status = twi_ack_rec ? TW_ST_LAST_DATA : TW_ST_DATA_NACK;
twi_onTwipEvent(twip_status);
twip_mode = TWIPM_WAIT;
twip_state = TWIP_WAIT_STOP;
}
// we have no more data to send and/or the master doesn't want anymore
twip_status = twi_ack_rec ? TW_ST_LAST_DATA : TW_ST_DATA_NACK;
twi_onTwipEvent(twip_status);
twip_mode = TWIPM_WAIT;
twip_state = TWIP_WAIT_STOP;
}
break;

default:
break;
}
}
}

Expand All @@ -687,9 +668,9 @@ void ICACHE_RAM_ATTR onSdaChange(void)
sda = SDA_READ();
scl = SCL_READ();

if (scl) /* !DATA */ switch (twip_state)
{
case TWIP_IDLE:
int twip_state_mask = S2M(twip_state);
if (scl) { /* !DATA */
IFSTATE(S2M(TWIP_IDLE)) {
if (sda) {
// STOP - ignore
} else {
Expand All @@ -698,27 +679,14 @@ void ICACHE_RAM_ATTR onSdaChange(void)
twip_state = TWIP_START;
ets_timer_arm_new(&timer, twi_timeout_ms, false, true); // Once, ms
}
break;

case TWIP_START:
case TWIP_REP_START:
case TWIP_SEND_ACK:
case TWIP_WAIT_ACK:
case TWIP_SLA_R:
case TWIP_REC_ACK:
case TWIP_READ_ACK:
case TWIP_RWAIT_ACK:
case TWIP_WRITE:
} else IFSTATE(S2M(TWIP_START)|S2M(TWIP_REP_START)|S2M(TWIP_SEND_ACK)|S2M(TWIP_WAIT_ACK)|S2M(TWIP_SLA_R)|S2M(TWIP_REC_ACK)|S2M(TWIP_READ_ACK)|S2M(TWIP_RWAIT_ACK)|S2M(TWIP_WRITE)) {
// START or STOP
SDA_HIGH(); // Should not be necessary
twip_status = TW_BUS_ERROR;
twi_onTwipEvent(twip_status);
twip_mode = TWIPM_WAIT;
twip_state = TWIP_BUS_ERR;
break;

case TWIP_WAIT_STOP:
case TWIP_BUS_ERR:
} else IFSTATE(S2M(TWIP_WAIT_STOP)|S2M(TWIP_BUS_ERR)) {
if (sda) {
// STOP
SCL_LOW(); // clock stretching
Expand All @@ -736,10 +704,7 @@ void ICACHE_RAM_ATTR onSdaChange(void)
ets_timer_arm_new(&timer, twi_timeout_ms, false, true); // Once, ms
}
}
break;

case TWIP_SLA_W:
case TWIP_READ:
} else IFSTATE(S2M(TWIP_SLA_W)|S2M(TWIP_READ)) {
// START or STOP
if (bitCount != 7) {
// inside byte transfer - error
Expand All @@ -765,10 +730,7 @@ void ICACHE_RAM_ATTR onSdaChange(void)
twip_mode = TWIPM_IDLE;
}
}
break;

default:
break;
}
}
}

Expand Down
0