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 Maker badge board #7135

Merged
merged 20 commits into from
Nov 11, 2022
Merged

Add Maker badge board #7135

merged 20 commits into from
Nov 11, 2022

Conversation

dronecz
Copy link

@dronecz dronecz commented Oct 26, 2022

Thanks🙂

@dronecz
Copy link
Author

dronecz commented Oct 30, 2022

Failed just at my board, which is expected as my PR is not merged. Am I right or should I look in to it again?

@dhalbert
Copy link
Collaborator

Failed just at my board, which is expected as my PR is not merged. Am I right or should I look in to it again?

No, your board should not fail. It looks like the sdkconfig file is missing: see https://github.com/adafruit/circuitpython/actions/runs/3357317522/jobs/5563100639.

But there are far too many changed files here. Don't add the frozen libraries directly by copying. Add them as git submodules.

@dronecz
Copy link
Author

dronecz commented Oct 31, 2022

Still unhappy about something 😕

Note: Not building languages {'hi', 'ko', 'cs', 'el'}
building boards with parallelism 2
Error: Build maker_badge for de_DE took 0.66s and failed
make: Entering directory '/home/runner/work/circuitpython/circuitpython/ports/espressif'
Use make V=1, make V=2 or set BUILD_VERBOSE similarly in your environment to increase build verbosity.
make: *** No rule to make target 'boards/maker_badge/sdkconfig', needed by 'build-maker_badge/esp-idf/config/sdkconfig.h'.  Stop.
make: *** Waiting for unfinished jobs....
mkdir -p build-maker_badge/genhdr
make: Leaving directory '/home/runner/work/circuitpython/circuitpython/ports/espressif'

Cannot find file ../ports/espressif/build-maker_badge/firmware.uf2

Should I add localy build firmware.uf2 file?

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

This pull request isn't building because espressif boards each need an "sdkconfig" file. The specific contents of the sdkconfig file depend on the espressif chip / module used.

Not all sdkconfig settings have to be in this file, but these do:

  • the size and speed of spiram
  • psram pinout information, in some cases

For example, this is the sdkconfig file for the kaluga devkit: https://github.com/adafruit/circuitpython/blob/main/ports/espressif/boards/espressif_kaluga_1.3/sdkconfig

I'm not 100% sure of the details of this file.


#define CIRCUITPY_BOOT_BUTTON (&pin_GPIO0)

#define BOARD_USER_SAFE_MODE_ACTION translate("pressing boot button at start up.\n")
Copy link
Member

Choose a reason for hiding this comment

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

The safe mode messages were recently re-worked. As a result, this line is not needed. Please remove this line. When CIRCUITPY_BOOT_BUTTON is defined, a predefined message is used:

            message = translate("The BOOT button was pressed at start up.\n");

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I removed that line 🙂

CIRCUITPY_ESP_FLASH_FREQ=40m
CIRCUITPY_ESP_FLASH_SIZE=4MB

CIRCUITPY_MODULE=wroom
Copy link
Member

Choose a reason for hiding this comment

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

CIRCUITPY_MODULE is no longer used. Remove this line.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I removed that line 🙂

Copy link
Member

Choose a reason for hiding this comment

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

An "IDF_TARGET" line is needed. For instance, if the specific microcontroller is esp32-s2 then it would be

IDF_TARGET = esp32s2

Also, can you verify that you did actually remove the unneeded lines I mentioned before?

Comment on lines 6 to 13
IDF_TARGET = esp32s2

INTERNAL_FLASH_FILESYSTEM = 1
LONGINT_IMPL = MPZ

# The default queue depth of 16 overflows on release builds,
# so increase it to 32.
CFLAGS += -DCFG_TUD_TASK_QUEUE_SZ=32
Copy link
Member

Choose a reason for hiding this comment

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

These lines are not needed, because these are defaults on espressif. Remove them please.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I removed those lines 🙂

@jepler
Copy link
Member

jepler commented Nov 2, 2022

Hi and thanks for the PR! I have some comments / requested changes. I'm sorry that I don't know enough about the sdkconfig file to guide you through getting the right content into it.

@dronecz
Copy link
Author

dronecz commented Nov 2, 2022

Thank you @jepler I could build localy so I was confused, why this build failed. I had sdkconfig file localy, but I do not know why, but I did not copy it to PR 🤦‍♂️. I hope this will build.

@jepler
Copy link
Member

jepler commented Nov 3, 2022

The failures you're seeing now are due to an incompatibility of esp-idf and python 3.11, which has started rolling out on github actions. I filed a PR to pin our python back at 3.10 during the build: #7164

@dronecz
Copy link
Author

dronecz commented Nov 3, 2022

The failures you're seeing now are due to an incompatibility of esp-idf and python 3.11, which has started rolling out on github actions. I filed a PR to pin our python back at 3.10 during the build: #7164

Thanks 🙂 Is there way, how can I restart build without making changes to my code?

@jepler
Copy link
Member

jepler commented Nov 3, 2022

I updated the PR to remove the change to circuitpython.pot, which is unneeded now anyway. This should cause a fresh re-builld with the fix to the python 3.11 problem too.

@@ -2,5 +2,5 @@ CONFIG_ESP32S2_SPIRAM_SUPPORT=n

# LWIP
#
CONFIG_LWIP_LOCAL_HOSTNAME="espressif"
CONFIG_LWIP_LOCAL_HOSTNAME="Maker_badge"
Copy link
Member

Choose a reason for hiding this comment

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

The filename is misspelled

Copy link
Author

Choose a reason for hiding this comment

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

Thank you! I missed that. 😩

@dronecz
Copy link
Author

dronecz commented Nov 9, 2022

@jepler It is again failing at language files. I really do not need circuitpython.pot file? Thanks

@jepler
Copy link
Member

jepler commented Nov 9, 2022

The build is failing for the following reason:

make: *** No rule to make target '../../frozen/Adafruit_CircuitPython_UC8151D', needed by 'build-maker_badge/frozen_mpy'.  Stop.

Additional steps are needed to properly add a new frozen module that has never been included before in any board. We have a guide on doing this via git on your local computer: https://learn.adafruit.com/building-circuitpython/adding-frozen-modules -- as far as I know you can't add a new submodule merely by the github website interface.

@jepler
Copy link
Member

jepler commented Nov 11, 2022

The build failed for the following reason:

subprocess.CalledProcessError: Command 'git describe --tags --exact-match' returned non-zero exit status 128.

as a policy, each frozen submodule must point exactly at a tag (release), not at another ref. There's a script for updating all libraries. I ran it and updated your branch, so maybe this time it will build successfully.

@dronecz
Copy link
Author

dronecz commented Nov 11, 2022

@jepler Many thanks for your help! I tried to fix it yesterday at my local build, but without success. Do you have more info about that script to fix this issue? I saw this information at page, which you linked earlier, but I did not figure out how to make it work.

image

It would be good to mention that script in the learn guide, so peple will not strugle to fix same issue.

@jepler
Copy link
Member

jepler commented Nov 11, 2022

The command is "make update-frozen-libraries" in the top level directory. I left feedback on the guide using the " Feedback? Corrections?" link so hopefully the maintainer will incorporate the suggestion soon.

Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

Thanks! It's taken a bit of time and I appreciate your responsiveness to my questions and suggestions. I think it's ready now.

CIRCUITPY_ESP_FLASH_FREQ=40m
CIRCUITPY_ESP_FLASH_SIZE=4MB

CIRCUITPY_MODULE=wroom
Copy link
Member

Choose a reason for hiding this comment

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

An "IDF_TARGET" line is needed. For instance, if the specific microcontroller is esp32-s2 then it would be

IDF_TARGET = esp32s2

Also, can you verify that you did actually remove the unneeded lines I mentioned before?

@jepler jepler merged commit 5e77269 into adafruit:main Nov 11, 2022
@dhalbert
Copy link
Collaborator

I left feedback on the guide using the " Feedback? Corrections?" link so hopefully the maintainer will incorporate the suggestion soon.

Done.

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.

None yet

3 participants