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

Minor fixes for esp32 configurations #27

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

philharrisathome
Copy link
Contributor

Minor fixes against the esp32 build:

  • Removed #define DISABLED & ENABLED in Constants.hpp due to clash with existing symbols in esp32 IDF. DISABLED & ENABLED don't appear anywhere else in the OAT codebase.
  • Removed unnecessary #include "Constants.hpp" in .\boards* . Constants is already included in the parent Configuration.hpp.
  • In Configuration.hpp default BOARD to new BOARD_UNKNOWN value and extend conditional tree at line 253. This is to prevent potential misconfigured hardware in case BOARD_TYPE is undefined: for an esp32 build, if BOARD_TYPE is undefined then Configuration.hpp would apply pin assignments for BOARD_AVR_MEGA2560. Mismatches were possible with other configurations also. With this change no pin assignments are made, however the configuration checks later in Configuration.hpp will provide a suitable message. @CTetford - you may want to review this.

@andre-stefanov
Copy link
Member

Mh ... could we sync the changes with hal ideas? Some of these changes already happened in the hal branch

@philharrisathome
Copy link
Contributor Author

I can take a look at making sure these are also merged/addressed into hal branch. It would also be worth merging these into devel for now to fix the warnings on esp32 there. Has all new development switched to hal? I'm still working with the premise that devel feeds release, and that at some point we will switch to hal.

@philharrisathome
Copy link
Contributor Author

Hi @andre-stefanov . Can we get this merged into develop? The build warnings and configuration conflicts fixed by this PR are still present.

@andre-stefanov andre-stefanov self-requested a review August 16, 2023 19:47
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