esp-idf-hal icon indicating copy to clipboard operation
esp-idf-hal copied to clipboard

Adds embedded_hal::blocking::i2c::Transactional

Open jmarcelomb opened this issue 2 years ago • 16 comments
trafficstars

Hello,

I was trying to use an ESP32-C3 with the module crate pn532. I didn't test yet, but I think it is the implementation.

Thank you

jmarcelomb avatar Apr 01 '23 18:04 jmarcelomb

Ty for the PR. Seems good so far, if you have the chance to test it with your sensor that would be great. Tracking Issue: rust-lang/cargo#197

Vollbrecht avatar Apr 01 '23 22:04 Vollbrecht

@jmmb13 can you fix the clippy warnings

Vollbrecht avatar Apr 03 '23 19:04 Vollbrecht

@Vollbrecht Is this still relevant?

ivmarkov avatar May 13 '23 10:05 ivmarkov

@Vollbrecht Is this still relevant?

yes, if the original author will not fix the CI i can impl it if you want.

Vollbrecht avatar May 13 '23 13:05 Vollbrecht

Let's do it, but no rush - it will be merged after this weekend's release anyway.

ivmarkov avatar May 13 '23 14:05 ivmarkov

I'm sorry for not answering, I was off. I added ";" that I think it is required to don't exit in the first operation.

jmarcelomb avatar May 13 '23 18:05 jmarcelomb

I'm sorry for not answering, I was off. I added ";" that I think it is required to don't exit in the first operation.

dont forget to run cargo fmt / cargo clippy to make sure ci dont fail

Vollbrecht avatar May 13 '23 18:05 Vollbrecht

I'm sorry for not answering, I was off. I added ";" that I think it is required to don't exit in the first operation.

dont forget to run cargo fmt / cargo clippy to make sure ci dont fail

How do I run clippy?

$ cargo clippy
   Compiling esp-idf-sys v0.33.0
error: failed to run custom build command for `esp-idf-sys v0.33.0`

Caused by:
  process didn't exit successfully: `/home/jmmb/Code/esp-idf-hal/target/debug/build/esp-idf-sys-dea13f6084fc101b/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-env-changed=ESP_IDF_TOOLS_INSTALL_DIR
  cargo:rerun-if-env-changed=ESP_IDF_SDKCONFIG
  cargo:rerun-if-env-changed=ESP_IDF_SDKCONFIG_DEFAULTS
  cargo:rerun-if-env-changed=MCU
  cargo:rerun-if-env-changed=ESP_IDF_SYS_ROOT_CRATE
  cargo:rerun-if-env-changed=ESP_IDF_VERSION
  cargo:rerun-if-env-changed=ESP_IDF_REPOSITORY
  cargo:rerun-if-env-changed=ESP_IDF_CMAKE_GENERATOR
  cargo:rerun-if-env-changed=IDF_PATH
  cargo:rerun-if-env-changed=EXTRA-COMPONENTS
  cargo:rerun-if-env-changed=ESP_IDF_COMPONENTS

  --- stderr
  Build configuration: BuildConfig {
      esp_idf_tools_install_dir: None,
      esp_idf_sdkconfig: None,
      esp_idf_sdkconfig_defaults: Some(
          [
              ".github/configs/sdkconfig.defaults",
          ],
      ),
      mcu: None,
      native: NativeConfig {
          esp_idf_version: None,
          esp_idf_repository: None,
          esp_idf_cmake_generator: None,
          idf_path: None,
          extra_components: [],
          esp_idf_components: None,
      },
      esp_idf_sys_root_crate: None,
  }
  Error: Unsupported target 'x86_64-unknown-linux-gnu'

jmarcelomb avatar May 13 '23 18:05 jmarcelomb

you can run the build commands like we do it in the CI : for example for an riscv target run cargo clippy --no-deps --target riscv32imc-esp-espidf -Zbuild-std=std,panic_abort -Zbuild-std-features=panic_immediate_abort -- -Dwarnings you can check all the build commands here : check Build | Fmt Check & Build | Clippy https://github.com/esp-rs/esp-idf-hal/actions/runs/4968078029/jobs/8890463099. the jobs on the left side are for all the different targets

Vollbrecht avatar May 13 '23 18:05 Vollbrecht

I ran it doesn't complain. Rust 1.69.0 and esp-idf v4.4.0

jmarcelomb avatar May 14 '23 14:05 jmarcelomb

@ivmarkov @MabezDev can you start the workflow

Vollbrecht avatar May 17 '23 19:05 Vollbrecht

I'm sorry, I was executing the cargo clippy command on the master branch, my bad. Now it is fixed :)

@ivmarkov @MabezDev, now you can start the workflow

jmarcelomb avatar May 21 '23 18:05 jmarcelomb

@Vollbrecht The PR passes fmt and clippy. Are there any other outstanding work, or shall I merge?

ivmarkov avatar Jun 11 '23 14:06 ivmarkov

@Vollbrecht The PR passes fmt and clippy. Are there any other outstanding work, or shall I merge?

@ivmarkov i think we are not ready yet. @jmmb13 i skimmed the documentation again for embedded-hal transaction. the way you propose to implement it is i think not compatible to the way it is described here. Calling the driver methods read/write in itself is a closed transaction with a separate start / stop bit send. Transactional needs to be implemented in a way so that the stop condition is only written once at the end. But each read / write will send it so you cant just use this methods.

@jmmb13 do you also know of a library that uses the e-hal exec method, so we can test against something when we implement it? your initial msg mentions pn532

Vollbrecht avatar Jun 12 '23 11:06 Vollbrecht

I can impl this trait in #397 for the legacy version of the driver.

teamplayer3 avatar Mar 25 '24 13:03 teamplayer3

I can impl this trait in #397 for the legacy version of the driver.

feel free to do so if you want!

Vollbrecht avatar Mar 25 '24 13:03 Vollbrecht