Added support for 2 new Olimex boards and updated 3 existing ones #9264

Stanimir-Petev · 2024-02-19T12:44:32Z

Hello!
Olimex employee here.

I have added support for the boards: Olimex ESP32-H2-DevKit-LiPo and ESP32-SBC-FabGL. Added a "pins_arduino.h" files in the "variants" folder and in the board.txt. The boards are open-source hardware and info can be found at our website.

Also for 3 of our existing in your package boards (ESP32-POE, ESP32-POE-ISO, ESP32-Gateway) inside the pins_arduino.h files I added the ethernet macros to match the fix from #9242 .

For ESP32-POE and ESP32-POE-ISO added extra menus and menu options for: PSRAM, Flash Size, Flash mode and Partition scheme.

I tested what I could but let me know if I have missed something and needs further changes.

Thanks espressif and Arduino team.

Added variants for ESP32-H2-DevKit-LiPo and ESP32-SBC-FabGL boards

Added menu for selection of the: 1) PSRAM enabled/disabled 2) Flash size 4/16 MB 3) Flash mode QIO/DIO 4) Partition Scheme additional options

Due to the latest changes in the ethernet library/examples (#9242) full package of definitions of the ethernet macros is for each board that has non-default values. For POE and POE-ISO is added a preprocessor condition #if defined BOARD_HAS_PSRAM due to our specific hardware because GPIO 16 and 17 are in use when PSRAM is enabled.

github-actions · 2024-02-19T12:45:13Z

	Warnings
⚠️	Some issues found for the commit messages in this PR: the commit message `"Added new Olimex boards"`: summary looks empty type/action looks empty the commit message `"Changes on the ESP32-POE and POE-ISO"`: summary looks empty type/action looks empty the commit message `"Update boards.txt"`: summary looks empty type/action looks empty the commit message `"Update boards.txt"`: summary looks empty type/action looks empty the commit message `"Update boards.txt"`: body's lines must not be longer than 100 characters summary looks empty type/action looks empty the commit message `"Updated pins_arduino.h for ESP32 POE, POE-ISO and Gateway"`: body's lines must not be longer than 100 characters summary looks empty type/action looks empty Please fix these commit messages - here are some basic tips: follow Conventional Commits style correct format of commit message should be: `<type/action>(<scope/component>): <summary>`, for example `fix(esp32): Fixed startup timeout issue` allowed types are: `change,ci,docs,feat,fix,refactor,remove,revert,test` sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) `TIP:` Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).
⚠️	Please consider squashing your 7 commits (simplifying branch history).

👋 Hello Stanimir-Petev, we appreciate your contribution to this project!

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against 36b9afd

P-R-O-C-H-Y · 2024-02-19T13:06:15Z

@Stanimir-Petev Everything looks fine :)

Jason2866 · 2024-02-19T14:51:01Z

@Stanimir-Petev The psram fixes are only needed when you use the very old esp32 revision 0 and 1. Since the OLIMEX ESP32-SBC-FABGL is a new design the can be removed.

Jason2866 · 2024-02-19T14:52:15Z

boards.txt

+esp32-sbc-fabgl.menu.PSRAM.disabled.build.defines=
+esp32-sbc-fabgl.menu.PSRAM.disabled.build.extra_libs=
+esp32-sbc-fabgl.menu.PSRAM.enabled=Enabled
+esp32-sbc-fabgl.menu.PSRAM.enabled.build.defines=-DBOARD_HAS_PSRAM -mfix-esp32-psram-cache-issue -mfix-esp32-psram-cache-strategy=memw


Psram fix not needed. Can be removed.

Hello @Jason2866 and thanks for the suggestion.

When I created the board entry in the board.txt I just copied that part from the generic ESP32 board (lines 1131-1318) because I have tested that it worked with it being selected. And after that I just changed the name name, build.variant and build.board.
I wasn't aware of what these mfix options does so I thought they are there for a reason and I just let them stay there.
If you are suggesting that they are unnecessary and can be safely removed that's all good.

The question I have is should I remove them from my forked repository and make a new pull request or you can do it directly on your repository?

Dont have the needed rights to change. Not a member -> Collaborator

@Stanimir-Petev just remove -mfix-esp32-psram-cache-issue -mfix-esp32-psram-cache-strategy=memw from all non-ESP32 boards. Those flags are valid only on the original ESP32

I have done it. I hope I did it correctly since I have never made a commit after a pull request was created and I am not sure if it breaks something.

Yes, since the esp32-sbc-fabgl uses ESP32-WROVER-E modules. And all this have eco3 esp32. https://www.olimex.com/Products/Retro-Computers/ESP32-SBC-FabGL/open-source-hardware

If that is true, then I can agree, but then why is PSRAM not enabled by default?

True, PSRAM should be always enabled!

No way to confirm if all ESP32-POE-WROVER and ESP32-POE-ISO-WROVER variants had version 3 modules. But these were made starting 2020 so good chance they are all version 3.

If you ask for all ESP32-POE and ESP32-POE-ISO boards - these use WROOM modules and had been in production since the early days of the ESP32.

We would enable the PSRAM for the ESP32-SBC_FABGL, for the rest the variants with WROOM module are still more popular and it makes more sense to have the PSRAM disabled. We don't want to create addition entries for each variant of the board, since some boards would need to have 3 entries like ESP32-POE, ESP32-POE-WROVER, ESP32-POE-16MB all the same design with different module soldered (one with WROVER module and PSRAM, and the one with bigger flash).

With the latest commit PSRAM enabled should be the default option on ESP32-SBC-FabGL. Is there anything else that has to be done?

Removed the -mfix-esp32-psram-cache-issue -mfix-esp32-psram-cache-strategy=memw fix flags as suggested by Jason2866 and me-no-dev

me-no-dev · 2024-02-20T10:52:19Z

boards.txt

+esp32-poe.menu.PSRAM.disabled.build.defines=
+esp32-poe.menu.PSRAM.disabled.build.extra_libs=
+esp32-poe.menu.PSRAM.enabled=Enabled (WROVER)
+esp32-poe.menu.PSRAM.enabled.build.defines=-DBOARD_HAS_PSRAM


here you need the workaround flags, because the board is ESP32 based

me-no-dev · 2024-02-20T10:52:53Z

boards.txt

+esp32-poe-iso.menu.PSRAM.disabled.build.defines=
+esp32-poe-iso.menu.PSRAM.disabled.build.extra_libs=
+esp32-poe-iso.menu.PSRAM.enabled=Enabled (WROVER)
+esp32-poe-iso.menu.PSRAM.enabled.build.defines=-DBOARD_HAS_PSRAM


flags also needed here

Reverted changes for ESP32-POE and ESP32-POE-ISO

Rearrange of the enable/disable entries of the PSRAM option for ESP32-SBC-FabGL to be enabled by default.

Stanimir-Petev added 3 commits February 19, 2024 14:06

Added new Olimex boards

ea34f46

Added variants for ESP32-H2-DevKit-LiPo and ESP32-SBC-FabGL boards

Changes on the ESP32-POE and POE-ISO

77b0955

Added menu for selection of the: 1) PSRAM enabled/disabled 2) Flash size 4/16 MB 3) Flash mode QIO/DIO 4) Partition Scheme additional options

P-R-O-C-H-Y added the Type: 3rd party Boards label Feb 19, 2024

P-R-O-C-H-Y approved these changes Feb 19, 2024

View reviewed changes

P-R-O-C-H-Y added the Status: Pending Merge Pull Request is ready to be merged label Feb 19, 2024

Jason2866 suggested changes Feb 19, 2024

View reviewed changes

Jason2866 removed the Status: Pending Merge Pull Request is ready to be merged label Feb 19, 2024

Update boards.txt

79449d9

Removed the -mfix-esp32-psram-cache-issue -mfix-esp32-psram-cache-strategy=memw fix flags as suggested by Jason2866 and me-no-dev

me-no-dev reviewed Feb 20, 2024

View reviewed changes

Stanimir-Petev and others added 3 commits February 20, 2024 13:05

Update boards.txt

a6dca0c

Reverted changes for ESP32-POE and ESP32-POE-ISO

Merge branch 'master' into master

e7da7ba

Update boards.txt

36b9afd

Rearrange of the enable/disable entries of the PSRAM option for ESP32-SBC-FabGL to be enabled by default.

me-no-dev merged commit e18d8b9 into espressif:master Feb 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for 2 new Olimex boards and updated 3 existing ones #9264

Added support for 2 new Olimex boards and updated 3 existing ones #9264

Stanimir-Petev commented Feb 19, 2024

github-actions bot commented Feb 19, 2024 •

edited

Loading

P-R-O-C-H-Y commented Feb 19, 2024

Jason2866 commented Feb 19, 2024

Jason2866 Feb 19, 2024

Stanimir-Petev Feb 20, 2024

Jason2866 Feb 20, 2024

me-no-dev Feb 20, 2024

Stanimir-Petev Feb 20, 2024

Jason2866 Feb 20, 2024 •

edited

Loading

me-no-dev Feb 20, 2024

Jason2866 Feb 20, 2024

DanKoloff Feb 20, 2024

Stanimir-Petev Feb 21, 2024

me-no-dev Feb 20, 2024

me-no-dev Feb 20, 2024

Added support for 2 new Olimex boards and updated 3 existing ones #9264

Added support for 2 new Olimex boards and updated 3 existing ones #9264

Conversation

Stanimir-Petev commented Feb 19, 2024

github-actions bot commented Feb 19, 2024 • edited Loading

P-R-O-C-H-Y commented Feb 19, 2024

Jason2866 commented Feb 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jason2866 Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 19, 2024 •

edited

Loading

Jason2866 Feb 20, 2024 •

edited

Loading