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

espulp module broken in CircuitPython 9 #8770

Closed
Sola85 opened this issue Dec 29, 2023 · 6 comments · Fixed by #9277
Closed

espulp module broken in CircuitPython 9 #8770

Sola85 opened this issue Dec 29, 2023 · 6 comments · Fixed by #9277
Labels
advanced api bug espressif applies to multiple Espressif chips third-party Awaiting action on a third party for a fix or an answer to a request
Milestone

Comments

@Sola85
Copy link

Sola85 commented Dec 29, 2023

CircuitPython version

Adafruit CircuitPython 9.0.0-alpha.6 on 2023-12-29; ESP32-S3-DevKitM-1-N8

Code/REPL

>>> import espulp
>>> ulp = espulp.ULP()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NotImplementedError:

Behavior

In CircuitPython 9 it is no longer possible to instantiate a ULP object. In CircuitPython 8 this still worked.

Description

The reason for this seems to be the new preprocessor directives that were introduced between CircuitPython 8 and 9:

#ifdef CONFIG_ULP_COPROC_TYPE_FSM
        case FSM:
            break;
#endif

In particular CONFIG_ULP_COPROC_TYPE_FSM does not seem to get defined anywhere.

I had started to work on improving the espulp module for CircuitPython 8 and wanted to port my changes to CircuitPython 9, but i hit this issue that i was not yet able to resolve.

Additional information

No response

@Sola85 Sola85 added the bug label Dec 29, 2023
@Sola85
Copy link
Author

Sola85 commented Dec 30, 2023

I have since found out that the location where CONFIG_ULP_COPROC_TYPE_FSM should be defined is in ports/espressif/esp-idf-config/sdkconfig-*.defaults.

The issue seems to be that idf.py menuconfig only allows either CONFIG_ULP_COPROC_TYPE_FSM or CONFIG_ULP_COPROC_TYPE_RISCV to be defined but not both at the same time. This is despite the ESP32S2 and S3 having both the FSM and RISCV ULPs...

It looks like this issue has to be fixed upstream in the espressif toolchain. In the meantime I would suggest to enable/disable FSM/RISCV support based on the IDF_TARGET and not based on CONFIG_ULP_COPROC_TYPE_FSM/CONFIG_ULP_COPROC_TYPE_RISCV

@Sola85
Copy link
Author

Sola85 commented Dec 30, 2023

According to the Reference Manual of the ESP32S3 (Page 29), the ULPs cannot be used simultaneously. This means that Espressif will likely not update idf.py menuconfig to allow CONFIG_ULP_COPROC_TYPE_FSM and CONFIG_ULP_COPROC_TYPE_RISCV to be enabled simultaneously.

But compiling Circuitpython with both ULPs worked for Circuitpython 8, so I suggest to revert the related changes from commit c38972b.
@microdev1 @tannewt What do you think about this?

@tannewt tannewt added advanced api espressif applies to multiple Espressif chips labels Jan 16, 2024
@tannewt tannewt added this to the 9.0.0 milestone Jan 16, 2024
@tannewt
Copy link
Member

tannewt commented Jan 16, 2024

I suspect you'll need to copy over the IDF functions we use so they are both available at compile time until espressif fixes it. I'd suggest filing an issue with them about making the API work dynamically like we use it.

@tannewt tannewt modified the milestones: 9.0.0, 9.x.x Feb 14, 2024
@tannewt tannewt added the third-party Awaiting action on a third party for a fix or an answer to a request label Feb 14, 2024
@Sola85
Copy link
Author

Sola85 commented Feb 28, 2024

It doesn't look like we will get a response from espressif soon. In the meantime I would propose to enable the FSM ULP for all boards. Then the API is consistent across all ESPs (unlike now, where RISCV is enabled on some and FSM on others).

@tannewt What do you think?

@tannewt
Copy link
Member

tannewt commented Feb 29, 2024

It doesn't look like we will get a response from espressif soon. In the meantime I would propose to enable the FSM ULP for all boards. Then the API is consistent across all ESPs (unlike now, where RISCV is enabled on some and FSM on others).

@tannewt What do you think?

If you want to get it working with FSM only that's fine with me. I'd be happy to review it. It isn't a priority for me at the moment.

@dhalbert
Copy link
Collaborator

dhalbert commented Jun 3, 2024

Fixed by #9277.

@dhalbert dhalbert closed this as completed Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
advanced api bug espressif applies to multiple Espressif chips third-party Awaiting action on a third party for a fix or an answer to a request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants