Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: Sane defaults #95

Closed
Vollbrecht opened this issue Apr 4, 2023 · 10 comments · Fixed by #100
Closed

Discussion: Sane defaults #95

Vollbrecht opened this issue Apr 4, 2023 · 10 comments · Fixed by #100

Comments

@Vollbrecht
Copy link
Collaborator

Vollbrecht commented Apr 4, 2023

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
Copy link
Collaborator Author

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
Copy link

andreyk0 commented Apr 8, 2023

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
Copy link
Collaborator Author

Vollbrecht commented Apr 8, 2023

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
Copy link

andreyk0 commented Apr 8, 2023

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
Copy link
Member

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:

  • Have only 2 prompts that always need to be answered: MCU and if you want to use default values
  • If the default values prompt is false, then ask the rest of the prompts.

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 ...

@SergioGasquez SergioGasquez linked a pull request Apr 14, 2023 that will close this issue
@Vollbrecht
Copy link
Collaborator Author

Vollbrecht commented Apr 14, 2023

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
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Member

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants