Skip to content
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.

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

User I2C support v2 #781

Merged
merged 10 commits into from
Sep 27, 2022
Merged

User I2C support v2 #781

merged 10 commits into from
Sep 27, 2022

Conversation

Daft-Freak
Copy link
Collaborator

A bit similar to #261. Allows inserting I2C transfers into the I2C state machine. Should probably bump the API version for this... Also has a few small cleanups.

Useful if you want to do something crazy like add a Pico W for wifi things. Mostly PR-ing this since the code exists and works.

@Daft-Freak
Copy link
Collaborator Author

I haven't decided if this is too much of a hack or not, but it is faster...

diff --git a/32blit-stm32/Src/32blit/i2c.cpp b/32blit-stm32/Src/32blit/i2c.cpp
index 679da217..53bd65df 100644
--- a/32blit-stm32/Src/32blit/i2c.cpp
+++ b/32blit-stm32/Src/32blit/i2c.cpp
@@ -166,11 +166,14 @@ namespace i2c {
         break;
 
       case SEND_REG_USER:
+        // this is a bit of a hack, set the delay before starting the user transfer
+        // to avoid affecting the timing
+        blit_i2c_delay(16, SEND_ACL);
+
         if(user_addr) {
           HAL_I2C_Master_Transmit_IT(&hi2c4, user_addr << 1, &user_reg, 1);
           i2c_state = user_buf_len ? DATA_USER : DONE_USER;
-        } else // skip user states
-          blit_i2c_delay(16, SEND_ACL);
+        }
         break;
 
       case DATA_USER:
@@ -189,7 +192,8 @@ namespace i2c {
         if(api.i2c_completed && !blit_user_code_disabled())
           api.i2c_completed(addr, user_reg, user_buf, user_buf_len);
 
-        blit_i2c_delay(16, SEND_ACL);
+        // alredy setup the next state/time
+        i2c_state = DELAY;
         break;
       }
     }

@Daft-Freak
Copy link
Collaborator Author

🤔 Maybe I should sneak the "raw serial" stuff from my micropython hacks in here too...

@Daft-Freak
Copy link
Collaborator Author

thinking Maybe I should sneak the "raw serial" stuff from my micropython hacks in here too...

Well, that happened (after rewriting it a bit).... this is a "low-level APIs" PR now I guess.

(Debug firmware is ~100 bytes too big...)

@Gadgetoid
Copy link
Contributor

(Debug firmware is ~100 bytes too big...)

Youch!

I wonder if it's worth ifdef guarding some of this stuff, since it's "advanced". Though that may consign it to obscurity.

@Daft-Freak
Copy link
Collaborator Author

I think the code size increase here is small, but debug builds were really close to the limit. Not needing to rebuild the firmware for my python repl would be nice 😄

Inspired ny the old user i2c PR. Done as part of the i2c state machine, with a callback when it's done.
That's a good way to hardfault
No point in continuing
The HAL_Master_(Transmit|Receive)_IT calls have two levels of busy check
It can only be OK or BUSY, and we already prevent BUSY
This is actually a small code size improvement from removing the static var
@Daft-Freak
Copy link
Collaborator Author

Now there's still >3k left in a debug build 😄

@Daft-Freak
Copy link
Collaborator Author

@Gadgetoid Gadgetoid merged commit e792895 into 32blit:master Sep 27, 2022
@Gadgetoid
Copy link
Contributor

MicroPython!!!! 😲

@Daft-Freak Daft-Freak deleted the user-i2c-2 branch September 27, 2022 16:21
@Daft-Freak
Copy link
Collaborator Author

I should really push the code for that somewhere to increase the chances of it getting finished/usable/not-entirely-forgotten 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants