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

esp_hal: Try to fix espflash command not found in HIL CI #1846

Closed
wants to merge 1 commit into from

Conversation

JurajSadel
Copy link
Contributor

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Testing

espflash is installed on all runners.
No idea how to test, manual trigger doesn't pick my branch.
This is fine and expected: https://github.com/esp-rs/esp-hal/actions/runs/10055905092/job/27793593075#step:3:21
However, this is weird https://github.com/esp-rs/esp-hal/actions/runs/10055905092/job/27793789788#step:2:20
I tried to explicitly point to my repo: main...JurajSadel:esp-hal:ci/espflash#diff-3f888d600c3bcb11dd1c4600c03a6249d5741149bbcb7d04bf3e7b037f5cc693R124-R128
But I still can't see that export step here: https://github.com/esp-rs/esp-hal/actions/runs/10056365902/job/27795198164#step:5:1
So I'm not sure how to test if my "fix" is working or not other than adding it to the merge queue

NOTE: I was testing with other branch because it is a mess, main thing here is the export PATH=$PATH:/home/espressif/.cargo/bin before espflash erase-flash

closes #1842

@JurajSadel JurajSadel added the skip-changelog No changelog modification needed label Jul 23, 2024
@MabezDev
Copy link
Member

Could you run a HIL work flow on your fork to see that it works?

Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

👍

@MabezDev MabezDev added this pull request to the merge queue Jul 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 23, 2024
@MabezDev
Copy link
Member

Could we add the --non-interactive flags to all espflash usages? This should solve the not a terminal errors

@MabezDev
Copy link
Member

Hmm, it seems non interactive mode is only for monitoring, we should specify the port instead then I think

@JurajSadel
Copy link
Contributor Author

Hmm, it seems non interactive mode is only for monitoring, we should specify the port instead then I think

We can try to set ESPFLASH_PORT env variable on the runner. Do we want to do that for S3 for now?

@MabezDev
Copy link
Member

Closed in favour of #1848

@MabezDev MabezDev closed this Jul 23, 2024
@JurajSadel JurajSadel deleted the fix/ci_espflash branch July 24, 2024 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog No changelog modification needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ensure espflash is installed on all runners
3 participants