I will just show you something:
Klipper3d:master ← nefelim4ag:i2c-async
opened 07:32PM - 30 Aug 24 UTC
I'm not a hardcore embedded guy - I can have wrong assumptions or understanding … here.
If I understood correctly.
For now, SPI/I2C is working in synchronous mode, which may shift other events in the scheduler queue.
I2C is slower in general because it is used with speeds: of 100kHz and 400kHz.
According to #6141, we can easily calculate timings, 5us per pulse, 18 pulses per byte + ack.
The average read request looks like 2 bytes write 6 byte read.
1 byte ~ 18 * 5 = 90 us
8 byte ~ 90 * 8 = 720 us
400kHz is 4x faster, so 22.5us and 180us with same load.
SPI is freaking fast, SW works as fast as pins go, HW looks like expected to run at 4Mhz.
So, 10 times faster, 1 pulse per bit ~ 0.25us.
0.25 * 8 = 2 us, scheduling per byte looks not so good here.
The toolhead moved at a speed of 200 mm/s, 40mm rotational distance and 32 micro steps should have 32us between steps: `1 / ((200 / 40) * 200 * 32)) ~ 31.25 us`
Because i2c is blocking it will affect other timings occasionally.
It feels important to me to honor TSTEP timing for TMC because they are used for interpolation/thresholds (stealth/coolstep).
I cannot argue that those timings create interpolation position lag or that is a reason why switching stealth/spread modes is unstable.
Current implementation often blocks and waits for each byte (HW)/bit(SW) just wasting CPU time.
Those graphs when my machine is at a standstill:
Current implementation BME680 on host, SHT3x on mcu. *HW i2c decreased mcu load to ~5.6%
Current implementation with disabled i2c sensors.
Such a difference in load is unexpected for me. I can guess it is calculated from time drift.
Because the actual Awake time difference is in the expected range (less than 1%).
Patched async I2C (MCU software):
mcu hw i2c bus:
Host MCU pthread:
---
Current master SW: `mcu_awake=0.006 mcu_task_avg=0.000013 mcu_task_stddev=0.000114`
Async I2C SW: `mcu: mcu_awake=0.001 mcu_task_avg=0.000001 mcu_task_stddev=0.000001`
---
New I2C SW timings with default 100k.
It looks like 6-7 us per pulse originated to a safe way of switching.
https://github.com/Klipper3d/klipper/pull/6141#discussion_r1201492770
The transition between bytes is a little suboptimal.
---
With bit time correction at 100kHz.
I was able to drive the bus around 500kHz, it looks like there is a limit on the GPIO toggle speed.
---
The main question is it worth it?
I tried different approaches, but for now, this is something more or less simple.
*Current i2c_read/write works as before.
The high-level Idea is to buffer klippy i2c requests and execute them in the background.
There is a new infrastructure defined around it.
Because i2c_read is currently used in different places, there is the possibility of defining a callback for i2c_async.
There is the ability to change ldc1612 and mpu9250, to use it with the nonblocking API.
Correct nonblocking HW i2c is written for stm32f0_i2c.c
Others I didn't touch for now, but I can also rewrite them - the event loop is the same.
Software i2c is tricky, I tried to not overcomplicate things and still emulate the bit-banging approach in tmcuart.c
(I unintentionally reinvented it somehow, with function overrides :D)
For now, the event loop is stored in each HW i2p implementation because it is easier to overcome specific restrictions, like:
- Linux HW i2s can only be made in one ioctl.
- AVR can be slow and such a thing will not work, so there count of jumps can be reduced.
This is not ready for merge.
TODO:
- ~~Make i2c_modify_bits async. There is Write-Read-Modify-Write cycle here.~~ - Done
- Write HW for other MCUs.
- i2c bus is shared, there is the possibility to send 2 async requests to the same bus - it will not work
- If there are decisions to get rid of old synchronous code here. I have a bad understanding of time measurements for i2c bulk data and how it will cooperate with async approach.
- ~~Does it make sense to have io pthread on Host MCU?~~ - that one reduce jitter
- ~~Commit mess should be fixed (I leave it for now as a history of thoughts).~~
---
MCU: Octopus Pro - stm32h723
Host: RPI 5
Klipper3d:master ← nefelim4ag:i2c-error-handling
opened 08:24PM - 14 Sep 24 UTC
As mentioned: https://github.com/Klipper3d/klipper/pull/6674#issuecomment-234466… 9008
That would be nice to handle I2C errors, instead of the current shutdown approach.
First 4 patches, actually from: #6684, #6687
Because one simplifies things, the other one adds actual errors instead of ignoring them.
Otherwise,
There is a refactoring part to make things more obvious, where is the dev and where is the bus.
Errors handling works, but:
- Some MCUs do not clearly state what happens underneath.
- There should be a better way to define & pass errors to Klippy. I think it would be possible to reuse static_stings_id for that. But I didn't find a simple way for that.
- When klippy inits and I2C returns an error, it is an unhandled exception, so it only appears in logs. There should be a fix actually to show it to frontends as before with shutdown msg.
- Missing handling for ldc1612/mpu9250 - they just ignore errors in runtime.
- Most sensors for now do something like:
```
def _sample():
try:
<code here>
except Exception:
logging.exception(...)
self.temp = self.humidity = .0
return self.reactor.NEVER
```
So, they stop updating data, if there is no heater for them, the machine will continue running.
---
About the code itself:
- Refactoring is done for clarity, it can be dropped or moved to separate PR.
(I still somewhat think about non-blocking i2c, it feels right to store queue/buf at bus level),
- `int ret` looks a little dirty to me, maybe is a bad approach here, I will be glad to receive any feedback.
Thanks.
The basic issue with PICO, is that it lacks of any error reporting for I2C. rp2040/i2c.c: Check for NACK/Start NACK by nefelim4ag · Pull Request #6692 · Klipper3d/klipper · GitHub (thanks)
If it works, It can be merged to klipper which should give a more or less clear error message in general with RP2040 and I2C.
SHT15 and SHT21 are different sensors.
In general, dupon are loose and just making them tighter fit could help with I2C stability.
1 Like