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

fix: Geekble Nano board setup update #11131

Merged
merged 9 commits into from
Mar 27, 2025
Merged

Conversation

SooDragon
Copy link
Contributor

@SooDragon SooDragon commented Mar 17, 2025

Geekble Nano board setup update

Description of Change

Geekble Nano board setup update

fix log : Changed Board name : Geekble_Nano_ESP32S3 to Geekble_nano_ESP32S3 to resolve Tools menu error
OldName_ToolsMenu
for Old name "Geekble_Nano_ESP32S3", Compile Error and Tools Menu Error Occurs.

NewName_Serial
NewName_ToolsMenu
NewName_Upload
for New name "Geekble_nano_Esp32S3", Errors are Corrected.

Tests scenarios

Tested under Arduino IDE 2.3.4 with Geekble nano 2.0 Dev Board

Related links

Geekble Nano board setup update
Copy link
Contributor

github-actions bot commented Mar 17, 2025

Warnings
⚠️

Some issues found for the commit messages in this PR:

  • the commit message "Geekble Nano board setup update":
    • 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 10 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).

👋 Hello SooDragon, we appreciate your contribution to this project!


📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

🖊️ Please also make sure you have read and signed the Contributor License Agreement for 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 756b135

Copy link
Member

@P-R-O-C-H-Y P-R-O-C-H-Y left a comment

Choose a reason for hiding this comment

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

Can you please clear the commented lines, so it's clear what are the changes?
Also keep the old name Geekble_Nano_ESP32S3, if there is not a good reason for that change.

@SooDragon
Copy link
Contributor Author

In my environment, the old name Geekble_Nano_ESP32S3 resulted Errors, but new name Geekble_nano_ESP32S3 is fine.
I tested it agian but in Arduino IDE 2.3.4, it's same.
It's weird but Geekble_nano_ESP32S3 works, Geekble_Nano_ESP32S3 troubles

may I keep the new name?

and also, reason for commented line was for Heavy-Users.
I removed most of Options in Tools menu for beginners.
but for Heavy Users, I thought they need some of the options I removed for beginners.
So, I left it Commented instead of erasing it, might helpful for Heavy users who want to edit and configure by them selves.

may I also keep the commented lines please?

Thank you! @P-R-O-C-H-Y

@P-R-O-C-H-Y
Copy link
Member

Can you add some log with the other naming (original) which is having troubles?

@P-R-O-C-H-Y
Copy link
Member

About the commented lines, please remove those to keep it clear. None of the boards have huge commented parts.
You can add to this PR a link to your repo with the "custom" board definition having all those options. Or keep atlas's some of the most used ones :)

fix: erase Comment Line
@SooDragon
Copy link
Contributor Author

@P-R-O-C-H-Y I erased the comment lines and and added logs. Thank you!

@Jason2866
Copy link
Collaborator

Where is the log file?

@SooDragon
Copy link
Contributor Author

Where is the log file?

On the description :)

@P-R-O-C-H-Y
Copy link
Member

@SooDragon Please check mine and @Jason2866 comment.

@P-R-O-C-H-Y
Copy link
Member

Where is the log file?

On the description :)

I dont see any error log there, which is reasoning the name change.

@SooDragon
Copy link
Contributor Author

Where is the log file?

On the description :)

I dont see any error log there, which is reasoning the name change.

fix log : Changed Board name : Geekble_Nano_ESP32S3 to Geekble_nano_ESP32S3 to resolve Tools menu error

was What I added, sorry for repeated requests, is there any suggestions or sample for adding error log?
I thought what I commented was a log for the error

@Jason2866
Copy link
Collaborator

For the boards name change there needs to be a reason. Post the log showing it fails using the not changed original board name.

@SooDragon
Copy link
Contributor Author

For the boards name change there needs to be a reason. Post the log showing it fails using the not changed original board name.

I Updated the log!
Could you please check it out?

@P-R-O-C-H-Y
Copy link
Member

I have no issues with compiling the Geekble nano S3 as it is now without your changes.

@SooDragon
Copy link
Contributor Author

The changes are focused on Helping Arduino Beginners, So with my changes, They will see less Tools options might confuse them :)

@P-R-O-C-H-Y
Copy link
Member

OK so please revert the name change and remove only the necessary stuff you would like to hide from users :)

@SooDragon
Copy link
Contributor Author

Yes It's applied could you check it out?

  1. commented lines are removed
  2. name is changed to resolve error described above
  3. removed unnecessary Tools menu so not to confuse Arduino Beginners
    Those 3 had been applied :)

@P-R-O-C-H-Y
Copy link
Member

Yes It's applied could you check it out?

  1. commented lines are removed
  2. name is changed to resolve error described above
  3. removed unnecessary Tools menu so not to confuse Arduino Beginners
    Those 3 had been applied :)

@SooDragon You are still renaming, which is not necessary as I messaged. I did test the board with the "old" name and there is no issue. So please revert the naming back and do only the 1. and 3. point for your comment. Also please check again if the variant folder name matches the variant name in boards.txt

@P-R-O-C-H-Y P-R-O-C-H-Y added Status: Awaiting Response awaiting a response from the author and removed Resolution: Awaiting response Waiting for response of author labels Mar 20, 2025
revert re-naming
@SooDragon
Copy link
Contributor Author

Thank you for your patience and support.
I changed following items according to your comments.

  1. removed re-naming and back to old name Geekble_Nano_ESP32S3
  2. checked variants folder name, and it was "Geekble_Nano_ESP32S3" so it's appliedGeekble_Nano_ESP32S3.build.variant=Geekble_Nano_ESP32S3

Thank you! @P-R-O-C-H-Y

@P-R-O-C-H-Y P-R-O-C-H-Y added Status: Pending Merge Pull Request is ready to be merged and removed Status: Awaiting Response awaiting a response from the author labels Mar 27, 2025
@me-no-dev me-no-dev merged commit bbaabb1 into espressif:master Mar 27, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Pending Merge Pull Request is ready to be merged Type: 3rd party Boards
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants