Skip to content

Ran Through Pins.c and really removed Trailing Whitepace#7521

Closed
BrainBoardz wants to merge 7 commits into
adafruit:mainfrom
BrainBoardz:main
Closed

Ran Through Pins.c and really removed Trailing Whitepace#7521
BrainBoardz wants to merge 7 commits into
adafruit:mainfrom
BrainBoardz:main

Conversation

@BrainBoardz
Copy link
Copy Markdown

Ran Through Pins.c and really removed Trailing Whitepace

Copy link
Copy Markdown
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thanks! Just some cosmetic changes.

You can use pre-commit to check the formatting in advance, to avoid whitespace errors and the like. See https://learn.adafruit.com/building-circuitpython/build-circuitpython#install-pre-commit-3096511

Comment on lines +39 to +48
bool board_requests_safe_mode(void) {
return false;
}

void reset_board(void) {

}

void board_deinit(void) {
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These routines are not needed if they do nothing. There are MP_WEAK default versions that substitute for them.

Suggested change
bool board_requests_safe_mode(void) {
return false;
}
void reset_board(void) {
}
void board_deinit(void) {
}
// Use the MP_WEAK supervisor/shared/board.c versions of routines not defined here.

Comment on lines +42 to +51
{ MP_ROM_QSTR(MP_QSTR_TX), MP_ROM_PTR(&pin_GPIO43) },
{ MP_ROM_QSTR(MP_QSTR_RX), MP_ROM_PTR(&pin_GPIO44) },
{ MP_ROM_QSTR(MP_QSTR_SDA), MP_ROM_PTR(&pin_GPIO8) },
{ MP_ROM_QSTR(MP_QSTR_SCL), MP_ROM_PTR(&pin_GPIO9) },
{ MP_ROM_QSTR(MP_QSTR_SD_MISO), MP_ROM_PTR(&pin_GPIO13) },
{ MP_ROM_QSTR(MP_QSTR_SD_MOSI), MP_ROM_PTR(&pin_GPIO15) },
{ MP_ROM_QSTR(MP_QSTR_SD_CLK), MP_ROM_PTR(&pin_GPIO14) },
{ MP_ROM_QSTR(MP_QSTR_SD_CS), MP_ROM_PTR(&pin_GPIO16) },
{ MP_ROM_QSTR(MP_QSTR_LED), MP_ROM_PTR(&pin_GPIO21) },
{ MP_ROM_QSTR(MP_QSTR_BOOT), MP_ROM_PTR(&pin_GPIO0) },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When there are two names for one pin, we usually group them together with whitespace so that's easy to spot. Put the name that is primary (e.g., the board silkscreen) first: that will cause that name to be the preferred print name for the pin. E.g. if your board has TX on the silkscreen, do this:

    { MP_ROM_QSTR(MP_QSTR_TX), MP_ROM_PTR(&pin_GPIO43) },
    { MP_ROM_QSTR(MP_QSTR_IO43), MP_ROM_PTR(&pin_GPIO43) },

If IO43 is the primary name, reverse the order. In either case, group the "twins" together and surround them with a blank line. You can find examples on other board.

Comment on lines +8 to +13
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
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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
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

These have been moved to common mpconfigport.mk and can be removed.

@microdev1
Copy link
Copy Markdown
Collaborator

@BrainBoardz You don't have to make a new PR every time the commit history is changed.
Any changes you push to the PR's head branch (main in this case) will be synced in the PR.

@tannewt tannewt added the board New board or update to a single board label Jan 31, 2023
@BrainBoardz
Copy link
Copy Markdown
Author

Thanks for all the help with this. I will make the changes you have outlined. At the moment I am going to pull my request for my board as I am having problems with serial mode on my board. I think it is related to a recurring VID/PID issue I have been encountering. If I uses VID 0x0239A/PID 0x80C8 my board works perfectly in MU etc. But this was rejected for CP inclusion as a duplicated VID/PID was found. When I switch to 0x303A/0x90C8 it passes the PID verification but the I get a serial error when trying to use MU (on some machines!). It's very confusing. I'm not sure if it is a case of some systems remembering previous PID/VIDs and locking out serial and I am investigating that.

In the meantime, I've contact Espressif to request an additional set of PIDs specific to my ESP32-S3 based Neuron. It will probably be named NeuronS3. If I re-submit it will likely be with this info.

@dhalbert
Copy link
Copy Markdown
Collaborator

Got it. Mu uses https://github.com/adafruit/Adafruit_Board_Toolkit (which we developed) to find the serial ports. How that is done varies per operating system.

If this is Windows, try Uwe Seiber's USB Cleanup tool, which can get rid of stale devices: https://learn.adafruit.com/welcome-to-circuitpython/troubleshooting#device-errors-or-problems-on-windows-3094694

@BrainBoardz
Copy link
Copy Markdown
Author

BrainBoardz commented Jan 31, 2023 via email

@BrainBoardz
Copy link
Copy Markdown
Author

BrainBoardz commented Jan 31, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

board New board or update to a single board

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants