esp-rs / esp-idf-template

A "Hello, world!" template of a Rust binary crate for the ESP-IDF framework.
373 stars 44 forks source link

Discussion: Sane defaults #95

Closed Vollbrecht closed 1 year ago

Vollbrecht commented 1 year ago

In lights of PR #87 and adding more options for #94 i would like to talk about:

  1. How to make it as painless as possible for new users ?
  2. Still give rich options for people with different needs.

To point 1 -> If I was a new (maybe not experienced) user and just would like to generate an empty hello-world, I would be discouraged by to many options. ( Asking what is wokwi / what dev containers etc ). I imagine to have a simple option like a --default flag that skips all the questions for the first time user. Have to look up if this can be done with cargo-generate

Why is the active "unstable" master branch called mainline? I personally don't like the name, hard to figure out for a new user.

Furthermore I would ask you what do you think should be the default path for all the options ? This question is especially geared towards the std and wokwi questions. Should the generated path represent a minimal generated project or a more complete?

Vollbrecht commented 1 year ago

In the sense of completeness. I personally also would like to include embedded-hal + svc if the user choose the std + esp-idf-hal / svc . And on top of it an async option would also be nice, but this is more debatable.

andreyk0 commented 1 year ago

Speaking of defaults, I needed to add CONFIG_ESP_CONSOLE_USB_CDC=y to sdkconfig.defaults to get it to print hello world on a ESP32-S2 mini board.

Perhaps that should be included?

Vollbrecht commented 1 year ago

Speaking of defaults, I needed to add CONFIG_ESP_CONSOLE_USB_CDC=y to sdkconfig.defaults to get it to print hello world on a ESP32-S2 mini board.

Perhaps that should be included?

Thanks for mention it. Though this is a somewhat harder issue. The core problem boils down that different board manufactures can take different decisions here coupled with the default ( old way) of the default way of esp-idf using usb to uart ftdi chip. The S2 is i think special in that regard that it is a bit different than the s3 /c3 on how the USB is integrated. Though could be wrong here. In short this option only needs to be set on boards where pin 19/20 are directly connected to the usb connector. There might be other boards where this is not the case. Question becomes is how to integrate it to make it for all users ( with ftdi chip and with direct usb connection) workable without question them about a setting that not all ( new ) users might understand.

andreyk0 commented 1 year ago

Thanks! I see. One way, to handle that (if that's not too verbose/annoying for all users) is to include some commented out sections in sdkconfig.defaults with suggestions to uncomment this or that depending on the board.

SergioGasquez commented 1 year ago

I really like the idea of having a “simplified” version of the template where newbie users don't have to go through a lot of prompts which they may not even know what they are to get to a simple working project. I can try to implement the following:

Example of newbie path:

❯ cargo generate -v --path ../../forks/esp-idf-template/ cargo
🤷   Project Name: newbie
🔧   Destination: /home/sergio/Documents/Espressif/tests/esp-idf-template/newbie ...
🔧   project-name: newbie ...
🔧   Generating template ...
✔ 🤷   Use default values? · true
✔ 🤷   Which MCU to target? · esp32
...
✨   Done! New project created ...

Example of expert path:

❯ cargo generate -v --path ../../forks/esp-idf-template/ cargo 
🤷   Project Name: expert
🔧   Destination: /home/sergio/Documents/Espressif/tests/esp-idf-template/expert ...
🔧   project-name: expert ...
🔧   Generating template ...
✔ 🤷   Use default values? · false
✔ 🤷   Which MCU to target? · esp32
✔ 🤷   Enable STD support? · true
✔ 🤷   Configure project to support Wokwi simulation with Wokwi VS Code extension? · true
✔ 🤷   Configure project to use Dev Containers (VS Code and GitHub Codespaces)? · true
✔ 🤷   ESP-IDF version (mainline = UNSTABLE) · v4.4
✔ 🤷   Include esp-idf-hal/esp-idf-svc? · Yes (default features)
....
✨   Done! New project created ...
Vollbrecht commented 1 year ago

again asking the question, why does the git master branch option called "mainline" == unstable. both words are somewhat correct but don't transport the information what target this really is, thought this is maybe only my perspective? maybe call it git master / git mainline / git HEAD or something with emphasize on the git part ?

SergioGasquez commented 1 year ago

IIRC, before, it was called master and many users were using it (and encountering issues), so we decided to rename it to mainline and add warnings about it being unstable. Here is the issue and PR

Vollbrecht commented 1 year ago

yeah but this is than just security through obscurity of some sorts? i think the unstable information is good, but it also should be clear to the user what it is.

SergioGasquez commented 1 year ago

yeah but this is than just security through obscurity of some sorts? i think the unstable information is good, but it also should be clear to the user what it is.

I am ok changing it to master if the esp-idf version prompt lives after the default prompt. If you are modifying all the prompts, we can rely on the user to know what it's doing (and there will also be the unstable warning)

SergioGasquez commented 1 year ago

I added a note to discuss changing mainline to master in the next community meeting!