Skip to content

bricks: fix mp_hal_stdout_tx_strn() signature mismatch#2

Merged
morita5840 merged 4 commits intospike-rtfrom
fix/mphal-stdout-tx-strn-signature
Mar 21, 2026
Merged

bricks: fix mp_hal_stdout_tx_strn() signature mismatch#2
morita5840 merged 4 commits intospike-rtfrom
fix/mphal-stdout-tx-strn-signature

Conversation

@morita5840
Copy link

No description provided.

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 aligns multiple Pybricks “bricks” ports with the expected mp_hal_stdout_tx_strn() function signature by updating it to return a byte count (mp_uint_t) and standardizing the length parameter to size_t.

Changes:

  • Update mp_hal_stdout_tx_strn() signatures from void to mp_uint_t and adjust parameter types to size_t across affected ports.
  • Return the number of bytes written (or written-so-far on error) instead of returning nothing / always returning 0.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
bricks/primehub_spike-rt/mphalport.c Update stdout TX API to return bytes written for both debug and Bluetooth paths.
bricks/nxt/mphalport.c Update stdout TX signature and return value (0 when disconnected/empty; otherwise full length).
bricks/ev3rt/mphalport.c Update stdout TX signature and return bytes written when Bluetooth is connected.
bricks/ev3dev/ev3dev_mphal.c Update stdout TX signature and now returns write() result (but needs type/robustness fix).
bricks/_common_stm32/mphalport.c Update stdout TX signature and return values for UART and Bluetooth paths.

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

Comment on lines +112 to +116
mp_uint_t mp_hal_stdout_tx_strn(const char *str, size_t len) {
MP_THREAD_GIL_EXIT();
int ret = write(STDOUT_FILENO, str, len);
MP_THREAD_GIL_ENTER();
(void)ret; // to suppress compiler warning
return ret < 0 ? 0 : ret;
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

write() returns ssize_t, but the result is stored in an int and then returned as mp_uint_t. On 64-bit systems this can truncate successful writes (and makes it impossible to correctly represent large counts). Use ssize_t for ret and return a properly converted byte count (and ideally loop to handle partial writes/EINTR so stdout output isn’t silently dropped).

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

@copilot プルリクエストを新規に作成するのではなく、このプルリクエストにコミットしてください

Copy link

Copilot AI commented Mar 21, 2026

@morita5840 I've opened a new pull request, #3, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link

Copilot AI commented Mar 21, 2026

@morita5840 I've opened a new pull request, #4, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits March 21, 2026 10:57
Fix mp_hal_stdout_tx_strn(): use ssize_t, loop for partial writes, handle EINTR
@morita5840 morita5840 merged commit 03ba5e9 into spike-rt Mar 21, 2026
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.

3 participants