Update of the spi driver so the Frimware works with the latest esp-idf 3.0

SHA2017-badge / Firmware

ESP32 firmware for the SHA2017 badge

https://wiki.sha2017.org/w/Badge

Other

83 stars 36 forks source link

Update of the spi driver so the Frimware works with the latest esp-idf 3.0 #214

Closed krzychb closed 6 years ago

krzychb commented 6 years ago

Ref: https://github.com/SHA2017-badge/Firmware/issues/213

Tested with:

https://github.com/espressif/esp-idf/commit/f8bda324ecde58214aaa00ab5e0da5eea9942aaf

To Do

Update the version of the esp-idf submodule of the 'Firmware' repository to https://github.com/espressif/esp-idf/commit/f8bda324ecde58214aaa00ab5e0da5eea9942aaf

Possible Improvements

Change badge_eink_dev_write_command_stream_u32 to really stream the data as badge_eink_dev_write_command_stream does
resolve compilation warnings on deprecated functions, unused variables, etc.

raboof commented 6 years ago

Travis reports:

/home/travis/build/SHA2017-badge/Firmware/components/badge/./badge_power.c: In function 'badge_battery_volt_sense':
/home/travis/build/SHA2017-badge/Firmware/components/badge/./badge_power.c:27:12: error: implicit declaration of function 'adc1_get_raw' [-Werror=implicit-function-declaration]
  int val = adc1_get_raw(ADC1_CHAN_VBAT_SENSE);

krzychb commented 6 years ago

@raboof, thank you for looking into it.

This error is because function adc1_get_raw() has been introduced in version of esp-idf post what is currently in this repository. I have not been working with linked submodules and was not sure how to upgrade esp-idf @ 37156d5 to https://github.com/espressif/esp-idf/commit/f8bda324ecde58214aaa00ab5e0da5eea9942aaf in this PR. I will look into it again or please give me some hints if possible.

Optionally I can revert back to adc1_get_voltage(), but it is deprcated and generates extra warnings.

basvs commented 6 years ago

To start with: nice work! Thanks!

I think this code-improvement should also work with the older esp-idf code, so is it possible to split the patch into the spi-update and a separate patch to fix the esp-idf compatibility things?

About the _u32 methods: The amount of available memory is not very much. We already use a lot of memory in the micropython implementation. Then I found out there's more available available, but it is only allowed to access that memory as 32-bit dwords. That's why we have the _u32 methods. I really don't know if the hardware is allowed to access that memory on a byte level, but I'm afraid it's not.

krzychb commented 6 years ago

Wow, so many comments :) Thank you for review, they are all good and interesting points.

I will then revert some changes to minimize the number of modification and memory usage, and issue a separate PR to implement the "spi-update" to work with the existing esp-idf @ 37156d5

Then, if we are satisfied with result , I will return to this PR to handle the update to the latest esp-idf.

Does it work for you?