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

Set default CPU for SPI ISR #426

Closed
wants to merge 1 commit into from
Closed

Set default CPU for SPI ISR #426

wants to merge 1 commit into from

Conversation

artemsen
Copy link

Fixes the error during SPI initialization:

E (417) spi_master: spi_master_init_driver(236): invalid core id
W (417) LGFX: Failed to spi_bus_add_device.

Fixes the error during SPI initialization:
```
E (417) spi_master: spi_master_init_driver(236): invalid core id
W (417) LGFX: Failed to spi_bus_add_device.
```

Signed-off-by: Artem Senichev <[email protected]>
@artemsen
Copy link
Author

Looks like the CI process uses an old version of ESP-IDF.
I work with the the latest master which has this commit.

@tobozo
Copy link
Collaborator

tobozo commented Jul 16, 2023

hi, thanks for your contribution 👍

the build failed for every esp32 job, can you describe the successful environment you used so it can be added to the workflows?

isolating this change in a LFGX config block may avoid the pitfall of hitting incompatibility with older tested versions, it looks like isr_cpu_id hasn't always been a member of spi_bus_config_t:

error: 'struct spi_bus_config_t' has no member named 'isr_cpu_id'
         buscfg.isr_cpu_id = INTR_CPU_ID_AUTO;

in other situations (e.g. platformio), an extra include may be necessary to bring the missing define up to scope:

error: 'INTR_CPU_ID_AUTO' was not declared in this scope
         buscfg.isr_cpu_id = INTR_CPU_ID_AUTO;
                             ^~~~~~~~~~~~~~~~

please enable the workflows on your fork so you can verify that every test succeds before submitting, also target the 'develop' branch instead of the master branch.

@tobozo
Copy link
Collaborator

tobozo commented Jul 16, 2023

LovyanGFX doesn't support the master branch of esp-idf, only some of their releases when the version number makes sense and has been thoroughly tested, this excludes the beta version you linked.

Unfortunately v5.1 and 5.0.3 won't be added to the workflows until the release descriptions are disambiguated and evocate logic instead of version incest:

ESP-IDF v5.1 is a minor update for ESP-IDF v5.0. Release v5.1 is mostly compatible with apps written for ESP-IDF v5.0.

ESP-IDF v5.0.3 is a bugfix update for ESP-IDF v5.0.

@artemsen
Copy link
Author

LovyanGFX doesn't support the master branch of esp-idf

Ok. Just FYI, it works perfect with today's esp master, at least with WT32-SC01 =)

Unfortunately v5.1 and 5.0.3 won't be added to the workflows

Sorry, then I close this PR.

Thank you!

@artemsen artemsen closed this Jul 16, 2023
@tobozo
Copy link
Collaborator

tobozo commented Jul 21, 2023

FYI esp-idf 5.1 was added anyway to the CI tests on the develop branch

https://github.com/lovyan03/LovyanGFX/blob/develop/.github/workflows/IDFBuild.yml

@artemsen
Copy link
Author

I have enabled CI workflow for my fork, so all idf builds have passed, but all other (arduino, opencv, ...) has failed. I'm not sure if it's my fault =)
I can create PR to the develop branch to run check inside the origin CI.

@tobozo
Copy link
Collaborator

tobozo commented Jul 21, 2023

The opencv error is probably due to a cache miss, it just means the opencv install script is outdated and needs to be rewritten but may be ignored for this PR.

The ArduinoCIBuild has been disabled for long and isn't used any more so that error can be ignored too.

Creating a new PR from/to develop branches will sure be the best way to verify this 👍

lovyan03 added a commit that referenced this pull request Jul 21, 2023
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