Skip to content

Conversation

@elral
Copy link
Collaborator

@elral elral commented Oct 30, 2025

Description of changes

This PR adds the support for the Raspberry Pi Pico2.
Formfactor and pinout is the same as for version 2.
Advantage of Pico2 are the 5V tolerant I/O's.

elral added 30 commits August 16, 2024 06:36
@elral elral requested a review from DocMoebiuz as a code owner October 30, 2025 07:55
@github-actions
Copy link

Board and firmware folder for this pull request:
Mobiflight-Connector.zip

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for the Raspberry Pi Pico2 board to the MobiFlight firmware. The Pico2 shares the same form factor and pinout as the original Raspberry Pi Pico but offers 5V tolerant I/O pins as a key advantage. The implementation introduces custom shift register wrapper functions and platform-specific serial number generation for the RP2350 chipset.

Key Changes:

  • Introduces custom shiftInData() and shiftOutData() wrapper functions to replace Arduino's built-in shift register functions with platform-specific timing adjustments
  • Adds Pico2-specific unique ID generation using the Pico SDK's pico_get_unique_board_id() function
  • Creates new PlatformIO build environment and board configuration files for the Raspberry Pi Pico2

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/MF_Modules/MFShiftData.h New header file declaring custom shift register wrapper functions
src/MF_Modules/MFShiftData.cpp Implementation of custom shift register functions with platform-specific timing
src/MF_OutputShifter/MFOutputShifter.cpp Updated to use shiftOutData() wrapper instead of Arduino's shiftOut()
src/MF_InputShifter/MFInputShifter.cpp Updated to use shiftInData() wrapper instead of Arduino's shiftIn()
src/Config.cpp Added RP2350-specific serial number generation using Pico SDK unique board ID
platformio.ini Added new build environment configuration for Raspberry Pi Pico2
_Boards/raspberrypi_pico2.board.json New board configuration with hardware IDs, module limits, and pin definitions
_Boards/raspberry_pico2.png Board image asset for Pico2

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +24 to +40
uint8_t i;

for (i = 0; i < 8; i++) {
if (bitOrder == LSBFIRST) {
digitalWrite(dataPin, val & 1);
val >>= 1;
} else {
digitalWrite(dataPin, (val & 128) != 0);
val <<= 1;
}

digitalWrite(clockPin, HIGH);
digitalWrite(clockPin, LOW);
#if !defined(ARDUINO_ARCH_AVR) && !defined(ARDUINO_ARCH_RP2040)
delayMicroseconds(1);
#endif
}
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

Inconsistent indentation detected in this function. Lines 24-40 use tabs for indentation, while lines 15-17 and 38-39 use spaces. The codebase should use consistent indentation throughout. Based on the existing code in the repository, spaces appear to be the standard. Please convert all tabs to spaces to maintain consistency.

Suggested change
uint8_t i;
for (i = 0; i < 8; i++) {
if (bitOrder == LSBFIRST) {
digitalWrite(dataPin, val & 1);
val >>= 1;
} else {
digitalWrite(dataPin, (val & 128) != 0);
val <<= 1;
}
digitalWrite(clockPin, HIGH);
digitalWrite(clockPin, LOW);
#if !defined(ARDUINO_ARCH_AVR) && !defined(ARDUINO_ARCH_RP2040)
delayMicroseconds(1);
#endif
}
uint8_t i;
for (i = 0; i < 8; i++) {
if (bitOrder == LSBFIRST) {
digitalWrite(dataPin, val & 1);
val >>= 1;
} else {
digitalWrite(dataPin, (val & 128) != 0);
val <<= 1;
}
digitalWrite(clockPin, HIGH);
digitalWrite(clockPin, LOW);
#if !defined(ARDUINO_ARCH_AVR) && !defined(ARDUINO_ARCH_RP2040)
delayMicroseconds(1);
#endif
}

Copilot uses AI. Check for mistakes.
${env.build_flags}
'-DMOBIFLIGHT_TYPE="MobiFlight RaspiPico2"'
'-DMOBIFLIGHT_NAME="MobiFlight RaspiPico2"'
-DMEMLEN_CONFIG=1496 ; MUST be less than EEPROM_SIZE!! MEM_OFFSET_CONFIG + MEM_LEN_CONFIG <= EEPROM_SIZE, see: eeprom_write_block (MEM_OFFSET_CONFIG, configBuffer, MEM_LEN_CONFIG);-D#define MEMLEN_NAMES_BUFFER 1000 // max. size for configBuffer, contains only names from inputs
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The comment on this line is malformed and contains extraneous text. It appears to combine multiple comment fragments. The text after the semicolon includes "-D#define MEMLEN_NAMES_BUFFER 1000 // max. size for configBuffer, contains only names from inputs" which should not be part of this comment. This should be simplified to just describe what MEMLEN_CONFIG is for, similar to the comment on line 176 for the original Pico configuration.

Suggested change
-DMEMLEN_CONFIG=1496 ; MUST be less than EEPROM_SIZE!! MEM_OFFSET_CONFIG + MEM_LEN_CONFIG <= EEPROM_SIZE, see: eeprom_write_block (MEM_OFFSET_CONFIG, configBuffer, MEM_LEN_CONFIG);-D#define MEMLEN_NAMES_BUFFER 1000 // max. size for configBuffer, contains only names from inputs
-DMEMLEN_CONFIG=1496 ; Size of config buffer, must be less than EEPROM_SIZE (see: eeprom_write_block)

Copilot uses AI. Check for mistakes.
Comment on lines +687 to +695
#if defined(RP2350_PSRAM_CS)
uint8_t UniqueID[UniqueIDsize];
pico_unique_board_id_t sn;
pico_get_unique_board_id(&sn);
for (size_t i = 0; i < UniqueIDsize; i++)
{
UniqueID[i] = sn.id[i];
}
#endif
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

Missing include directive for Pico SDK unique ID functions. The code uses pico_unique_board_id_t and pico_get_unique_board_id on lines 689-690, but the required header file is not included. You need to add an include directive for the pico/unique_id.h header within the RP2350_PSRAM_CS conditional block to ensure these types and functions are properly declared.

Copilot uses AI. Check for mistakes.
Comment on lines +687 to +695
#if defined(RP2350_PSRAM_CS)
uint8_t UniqueID[UniqueIDsize];
pico_unique_board_id_t sn;
pico_get_unique_board_id(&sn);
for (size_t i = 0; i < UniqueIDsize; i++)
{
UniqueID[i] = sn.id[i];
}
#endif
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

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

The RP2350_PSRAM_CS conditional block creates a local UniqueID array that is only in scope within lines 687-695. However, the code after this block (lines 699-704, outside this diff) uses the UniqueID array. For non-Pico2 platforms, UniqueID should come from the ArduinoUniqueID library that was conditionally excluded on line 14. This creates a logic error where Pico2 boards define their own local UniqueID but then it goes out of scope before being used. The UniqueID population and serial string formatting should be kept together within the same conditional block, or the conditional on line 14 needs to allow ArduinoUniqueID for RP2350_PSRAM_CS platforms as well.

Copilot uses AI. Check for mistakes.
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.

1 participant