Add new i2c includes for idf 5.2 #290
Conversation
src/include/esp-idf/bindings.h
Outdated
| #include "driver/i2c_types.h" | ||
| #include "driver/i2c_master.h" | ||
| #include "driver/i2c_slave.h" | ||
| #else | ||
| #include "driver/i2c.h" |
There was a problem hiding this comment.
Let's keep the old driver available with 5.2+?
There was a problem hiding this comment.
I read this The legacy driver can't coexist with the new driver. Include i2c.h to use the legacy driver or the other two headers to use the new driver. Please keep in mind that the legacy driver is now deprecated and will be removed in future. in the doc. Consequently it's a problem to include both if I understand it correctly. Or is this not a problem when we use bindgen?
Additionally it's a general problem to implement the embedded_hal for the new i2c impl. when the interface is not any more available. Should the trait implemented with unimplented!() and give a hint to use the legacy driver?
There was a problem hiding this comment.
its the same with the gptimer headers. Creating the bindgen itself is not a problem. Also "compiling" works, but as soon as you will run both, there will be runtime check at startup that enforces it and will crash your app. So its ok to have them both in esp-idf-sys, but a wrapper driver in esp-idf-hal would need to be conditionally included
There was a problem hiding this comment.
@teamplayer3 Not sure if it was clear: to merge this, we need you to remove the #else statement.
Challenges
i2cdoes not expose commands interface, so thetransactionfunction in theesp-idf-halcouldn't be used anymore.I2CDriverimplementation with the transaction interface. This will lead to#include hal/i2c_types.hbecause the legacy header exposes types from the hal. The newi2cimplementation could be used for an extraI2CBusDriver.i2cimplementation inesp-idf.