-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Picoplanet #3267
Picoplanet #3267
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just two minor things. After this, submit a PR to https://github.com/adafruit/circuitpython-org.
#define IGNORE_PIN_PB01 1 | ||
#define IGNORE_PIN_PB02 1 | ||
#define IGNORE_PIN_PB03 1 | ||
#define IGNORE_PIN_PB04 1 | ||
#define IGNORE_PIN_PB05 1 | ||
#define IGNORE_PIN_PB06 1 | ||
#define IGNORE_PIN_PB07 1 | ||
#define IGNORE_PIN_PB08 1 | ||
#define IGNORE_PIN_PB09 1 | ||
#define IGNORE_PIN_PB10 1 | ||
#define IGNORE_PIN_PB11 1 | ||
#define IGNORE_PIN_PB12 1 | ||
#define IGNORE_PIN_PB13 1 | ||
#define IGNORE_PIN_PB14 1 | ||
#define IGNORE_PIN_PB15 1 | ||
#define IGNORE_PIN_PB16 1 | ||
#define IGNORE_PIN_PB17 1 | ||
#define IGNORE_PIN_PB22 1 | ||
#define IGNORE_PIN_PB23 1 | ||
#define IGNORE_PIN_PB30 1 | ||
#define IGNORE_PIN_PB31 1 | ||
#define IGNORE_PIN_PB00 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need these because these pins are not even defined for SAMD21E variant.
|
||
|
||
{ MP_ROM_QSTR(MP_QSTR_A1), MP_ROM_PTR(&pin_PA02) }, | ||
{ MP_ROM_QSTR(MP_QSTR_A0), MP_ROM_PTR(&pin_PA03) }, | ||
{ MP_ROM_QSTR(MP_QSTR_A2), MP_ROM_PTR(&pin_PA04) }, | ||
|
||
|
||
{ MP_ROM_QSTR(MP_QSTR_D5),MP_ROM_PTR(&pin_PA05) }, | ||
{ MP_ROM_QSTR(MP_QSTR_D6),MP_ROM_PTR(&pin_PA06) }, | ||
{ MP_ROM_QSTR(MP_QSTR_D7),MP_ROM_PTR(&pin_PA07) }, | ||
|
||
|
||
{ MP_ROM_QSTR(MP_QSTR_D1), MP_ROM_PTR(&pin_PA08) }, | ||
{ MP_ROM_QSTR(MP_QSTR_A3), MP_ROM_PTR(&pin_PA08) }, | ||
{ MP_ROM_QSTR(MP_QSTR_SDA), MP_ROM_PTR(&pin_PA08) }, | ||
|
||
{ MP_ROM_QSTR(MP_QSTR_D2), MP_ROM_PTR(&pin_PA09) }, | ||
{ MP_ROM_QSTR(MP_QSTR_A4), MP_ROM_PTR(&pin_PA09) }, | ||
{ MP_ROM_QSTR(MP_QSTR_SCL), MP_ROM_PTR(&pin_PA09) }, | ||
|
||
{ MP_ROM_QSTR(MP_QSTR_D3), MP_ROM_PTR(&pin_PA16) }, | ||
{ MP_ROM_QSTR(MP_QSTR_MOSI), MP_ROM_PTR(&pin_PA16) }, | ||
|
||
{ MP_ROM_QSTR(MP_QSTR_D4), MP_ROM_PTR(&pin_PA17) }, | ||
{ MP_ROM_QSTR(MP_QSTR_SCK), MP_ROM_PTR(&pin_PA17) }, | ||
|
||
{ MP_ROM_QSTR(MP_QSTR_D8), MP_ROM_PTR(&pin_PA30) }, | ||
{ MP_ROM_QSTR(MP_QSTR_MISO), MP_ROM_PTR(&pin_PA30) }, | ||
|
||
{ MP_ROM_QSTR(MP_QSTR_I2C), MP_ROM_PTR(&board_i2c_obj) }, | ||
{ MP_ROM_QSTR(MP_QSTR_SPI), MP_ROM_PTR(&board_spi_obj) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as you're editing this, could you touch up the indentation here, and remove the double blank lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks so much for your feedback! I fixed indentation and double blank lines, but it seems pre-commit still fails. Did I miss something?
STATIC const mp_rom_map_elem_t board_global_dict_table[] = { | ||
{ MP_ROM_QSTR(MP_QSTR_A1), MP_ROM_PTR(&pin_PA02) }, | ||
{ MP_ROM_QSTR(MP_QSTR_A0), MP_ROM_PTR(&pin_PA03) }, | ||
{ MP_ROM_QSTR(MP_QSTR_A2), MP_ROM_PTR(&pin_PA04) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have trailing white space at the end of this line. The pre-commit check doesn't like it.
{ MP_ROM_QSTR(MP_QSTR_I2C), MP_ROM_PTR(&board_i2c_obj) }, | ||
{ MP_ROM_QSTR(MP_QSTR_SPI), MP_ROM_PTR(&board_spi_obj) } | ||
}; | ||
MP_DEFINE_CONST_DICT(board_module_globals, board_global_dict_table); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need a newline at the end of the file.
//#define CP_RGB_STATUS_G (&pin_PA05) | ||
//#define CP_RGB_STATUS_B (&pin_PA07) | ||
//#define CP_RGB_STATUS_INVERTED_PWM | ||
//#define CP_RGB_STATUS_LED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More trailing whitespace here.
{ MP_ROM_QSTR(MP_QSTR_D1), MP_ROM_PTR(&pin_PA08) }, | ||
{ MP_ROM_QSTR(MP_QSTR_A3), MP_ROM_PTR(&pin_PA08) }, | ||
{ MP_ROM_QSTR(MP_QSTR_SDA), MP_ROM_PTR(&pin_PA08) }, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it also doesn't like the spaces on the empty lines.
If you click on "details" by the failed test, you can see what failed exactly. It also shows you the diff of what needs to be changed. |
Check here and similar for the details about what's wrong. https://github.com/adafruit/circuitpython/runs/973297866 Which editor are you using? emacs and other editors can automatically remove trailing whitespace (and fix missing newlines) on save. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! Thanks for your patience with the pre-commit checker.
Thank you so much for your patience with me :) |
Hi, I'd like to add the description for the generative PicoPlanet boards :)