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

Add erase-flash option to erase the flash before programming. (RDT-234) #98

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

Ouss4
Copy link
Contributor

@Ouss4 Ouss4 commented Jun 29, 2022

Add an option to erase the flash before programming. And example that needs this is: espressif/arduino-esp32#6916
Tested with the Arduino example by adding:

diff --git a/examples/arduino/hello_world/test_hello_world.py b/examples/arduino/hello_world/test_hello_world.py
index 0e7cea8..03f7346 100644
--- a/examples/arduino/hello_world/test_hello_world.py
+++ b/examples/arduino/hello_world/test_hello_world.py
@@ -1,2 +1,9 @@
+import pytest
+
+@pytest.mark.parametrize(
+    'erase_flash',
+    ['True'],
+    indirect=True,
+)
 def test_hello_arduino(dut):
     dut.expect('Hello Arduino!')

IDF service added a similar function bu I am not sure how that one is used.

@hfudev
Copy link
Member

hfudev commented Jun 30, 2022

Hi @Ouss4! if I understand correctly, you're trying to erase the nvs right? (guess based on this). We already have a erase_nvs field for idf. could you use integrate erase_nvs to arduino? (instead add another cli option erase_all)

@github-actions github-actions bot changed the title Add erase-flash option to erase the flash before programming. Add erase-flash option to erase the flash before programming. (RDT-234) Jun 30, 2022
@Ouss4
Copy link
Contributor Author

Ouss4 commented Jun 30, 2022

Hi @Ouss4! if I understand correctly, you're trying to erase the nvs right? (guess based on this). We already have a erase_nvs field for idf. could you use integrate erase_nvs to arduino? (instead add another cli option erase_all)

Hi,
Well, that tests requires to only erase the NVS partition but it has been requested to erase the whole flash too. So I added this. I am not sure if it's needed somewhere else.

@Ouss4
Copy link
Contributor Author

Ouss4 commented Jun 30, 2022

BTW, @hfudev we do have a function in IDF service that erases the flash but it's not exposed, do you know where that one is used? I know it wasn't you who added it.

@hfudev
Copy link
Member

hfudev commented Jul 5, 2022

BTW, @hfudev we do have a function in IDF service that erases the flash but it's not exposed, do you know where that one is used? I know it wasn't you who added it.

like here: https://github.com/espressif/esp-idf/blob/master/examples/system/ota/advanced_https_ota/pytest_advanced_ota.py#L473

Copy link
Member

@hfudev hfudev left a comment

Choose a reason for hiding this comment

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

@Ouss4 LGTM! It seems useful. Do you mind add this cli option for idf service as well?

@Ouss4
Copy link
Contributor Author

Ouss4 commented Jul 5, 2022

Do you mind add this cli option for idf service as well?

I added it to IDF service. What to do with the function erase_flash? Should we delete it and use the parameter? This will require tests that use that function to be updated.

Signed-off-by: Abdelatif Guettouche <abdelatif.guettouche@espressif.com>
@hfudev
Copy link
Member

hfudev commented Jul 7, 2022

Do you mind add this cli option for idf service as well?

I added it to IDF service. What to do with the function erase_flash? Should we delete it and use the parameter? This will require tests that use that function to be updated.

let's keep the old erase_flash function, since if we remove this it would requires changing the test scripts in IDF.

@hfudev hfudev merged commit 6f245aa into espressif:main Jul 7, 2022
@hfudev
Copy link
Member

hfudev commented Jul 7, 2022

Thank you @Ouss4 !

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 this pull request may close these issues.

2 participants