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

Simplify onboarding (VSC-426) #159

Merged

Conversation

brianignacio5
Copy link
Collaborator

@brianignacio5 brianignacio5 commented Sep 1, 2020

This branch is built to simplify user setup window.

  • Cancellable downloads
  • Express setup: No customization of ESP-IDF Tools and python virtual environment, just allow to choose ESP-IDF folder and install tools on default IDF_TOOLS_PATH ~/.espressif or %USERPROFILE%/.espressif. If the user updates $IDF_TOOLS_PATH
  • Custom setup: Allows the user to select the IDF_TOOLS_PATH or manually input each required tool and environment variable.
  • Use default settings if existing ESP-IDF, ESP-IDF Tools and virtual environment are found. This will install Debug Adapter and Extension requirements in virtual environment found.

@brianignacio5 brianignacio5 marked this pull request as draft September 1, 2020 14:12
@github-actions github-actions bot changed the title Simplify onboarding Simplify onboarding (VSC-426) Sep 1, 2020
@brianignacio5 brianignacio5 marked this pull request as ready for review September 14, 2020 12:55
@brianignacio5 brianignacio5 self-assigned this Sep 16, 2020
@brianignacio5 brianignacio5 added the enhancement New feature or request label Sep 16, 2020
Copy link
Contributor

@pwmb pwmb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I select the folder with IDF already there and try to install latest version of idf its going to this next page and UI freezes, ideally if folder had IDF already IDF, show the warning to user that this will override any changes they have there, and they can press Yes or Continue to proceed

Screen Shot 2020-09-17 at 12 38 47 PM


Use existing ESP-IDF setup button is non functional!!


Old error is persisting, even after changing the directory (should be reactive)

Screen Shot 2020-09-17 at 12 42 43 PM

Same here

Screen Shot 2020-09-17 at 12 44 38 PM

@brianignacio5
Copy link
Collaborator Author

Hmm should redirected to install page if error happens. Will fix.

Use existing ESP-IDF setup button is non functional!!

Any error in the logs or vscode Developer Tools console ?

@pwmb pwmb changed the base branch from master to dev September 24, 2020 11:26
@pwmb pwmb added this to the next milestone Sep 24, 2020
@brianignacio5
Copy link
Collaborator Author

If I select the folder with IDF already there and try to install latest version of idf its going to this next page and UI freezes, ideally if folder had IDF already IDF, show the warning to user that this will override any changes they have there, and they can press Yes or Continue to proceed

Screen Shot 2020-09-17 at 12 38 47 PM

Use existing ESP-IDF setup button is non functional!!

Old error is persisting, even after changing the directory (should be reactive)

Screen Shot 2020-09-17 at 12 42 43 PM

Same here

Screen Shot 2020-09-17 at 12 44 38 PM

This should be fixed. Please check the other errors.

@pwmb pwmb changed the base branch from dev to master September 29, 2020 06:53
src/views/setup/Home.vue Show resolved Hide resolved
Copy link
Contributor

@pwmb pwmb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CSS Suggestions

  • Add a box/border around so that these options just don't like text
  • On mouse Hover on them change cursor to pointer to indicate link
  • Also on hover change the text to blue (which is already the current behavior)

Screen Shot 2020-10-14 at 6 17 49 PM

  • Left align the content in card (not center) and Right align the action buttons

Screen Shot 2020-10-14 at 6 23 10 PM

  • First screen for both Express & Advance are identical is this intensional ?

@brianignacio5
Copy link
Collaborator Author

CSS suggestions should have been fixed.

First screen for both Express & Advance are identical is this intensional ?

Yes because it is necessary for both advanced and express setup modes. Tools need ESP-IDF to be defined first in any case.

@kumekay
Copy link
Collaborator

kumekay commented Nov 16, 2020

I like the new download screen, it concise and informative. Great improvement.

My exprerience with the installer. Here are issues I encoutered on Windows 10 20h2 in WSL2 with ubuntu 20.04

What is the difference between advanced and express installation? Both of them lead to configuration page.
image

Switching animation between screens is too long. It feels unresponsive. 0.4 s will be enough.

Missing space for label:
image

May the install create full path automatically, if it doesn't exist? Error messages doesn't fit.
image

The error is still on the next step, even after I created the path manually and installation goes normally:
image

I randomly receive
image

Log doesn't contain anything useful:

xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.79% (83343.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.81% (83359.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.83% (83375.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.85% (83391.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.87% (83407.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.89% (83423.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.91% (83439.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.93% (83455.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.95% (83471.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.97% (83487.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 99.99% (83503.63 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 100.00% (83515.85 / 83515.85) KB

Another issue that download progress is too verbose.

On download page progress bar for xtensa-esp32-elf is filled with pervious attempt results.
image

With the second attempt it logged the error. Is codespace somehow hardcoded?

xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 100.00% (83515.85 / 83515.85) KB
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz has (83516) KB
Failed download. Retrying...
Error: Error: Error creating dist directory
xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz progress: 0.02% (15.63 / 83515.85) KB
Error: ENOENT: no such file or directory, open '/home/codespace/.espressif/dist/xtensa-esp32-elf-gcc8_2_0-esp-2020r2-linux-amd64.tar.gz'
Error during ESP-IDF Tools download

Wrong \ slash for WSL path
image

@kumekay
Copy link
Collaborator

kumekay commented Nov 16, 2020

On Windows directly, install button doesn't work. Screenshot with error is attached:

image

@brianignacio5
Copy link
Collaborator Author

What is the difference between advanced and express installation? Both of them lead to configuration page.

Advanced allows you to modify Tools directory folder or enter each tool path manually.

With the second attempt it logged the error. Is codespace somehow hardcoded?

No it is not. /home/codespace is obtained from $HOME, the default tools directory is $HOME/.espressif.

Fixed other issues mentioned by @kumekay Please take a look again when possible.

@pwmb pwmb modified the milestones: v0.6.0, next Dec 30, 2020
@pwmb pwmb modified the milestones: next, v0.6.0 Jan 4, 2021
@brianignacio5 brianignacio5 merged commit 7532f6c into espressif:master Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants