Skip to content

extmod/machine_signal: Add Signal print function. - #12291

Draft
IhorNehrutsa wants to merge 1 commit into
micropython:masterfrom
IhorNehrutsa:signal_repr
Draft

extmod/machine_signal: Add Signal print function.#12291
IhorNehrutsa wants to merge 1 commit into
micropython:masterfrom
IhorNehrutsa:signal_repr

Conversation

@IhorNehrutsa

@IhorNehrutsa IhorNehrutsa commented Aug 23, 2023

Copy link
Copy Markdown
Contributor

Test code is:

from machine import Pin, Signal
signal22= Signal(Pin(22, mode=Pin.OUT))
signal22
signal21= Signal(Pin(21, mode=Pin.IN), invert=True)
signal21

Ounput is:

Signal(Pin(22))
Signal(Pin(21), invert=True)

instead of

<Signal>
<Signal>

@github-actions

github-actions Bot commented Aug 23, 2023 •
edited
Loading

Copy link
Copy Markdown

Code size report:

Reference:  webassembly/library: Fix ccall ABI for mp_hal_get_interrupt_char. [a5bac1b]
Comparison: machine_signal: Add Signal print function. [merge of 500c453]
  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:   +88 +0.010% standard
      stm32:   +68 +0.017% PYBV10
      esp32:   +60 +0.003% ESP32_GENERIC[incl +16(data)]
     mimxrt:   +64 +0.016% TEENSY40
        rp2:   +56 +0.006% RPI_PICO_W
       samd:   +68 +0.024% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:   +71 +0.015% VIRT_RV32

@codecov

codecov Bot commented Aug 23, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.51%. Comparing base (ff2ed49) to head (500c453).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #12291   +/-   ##
=======================================
  Coverage   98.51%   98.51%           
=======================================
  Files         177      177           
  Lines       22992    22995    +3     
=======================================
+ Hits        22651    22654    +3     
  Misses        341      341

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@IhorNehrutsa

IhorNehrutsa commented Aug 25, 2023

Copy link
Copy Markdown
Contributor Author

@jimmo
Could you give your opinion about this PR?

jimmo
jimmo reviewed Sep 1, 2023
View reviewed changes

@jimmo jimmo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @IhorNehrutsa -- I think this is useful.

Having this is useful for debugging and we already do it for Pin, so Signal should match. If all this printing support is too much code size we could consider making it a build option (e.g. MICROPY_PY_MACHINE_DETAILED_PRINTING) similar to how we do terse/detailed error messages.

Comment thread extmod/machine_signal.c Outdated
@IhorNehrutsa
IhorNehrutsa requested a review from jimmo September 1, 2023 07:43
@dpgeorge dpgeorge added the extmod Relates to extmod/ directory in source label Oct 11, 2023
@Gadgetoid Gadgetoid mentioned this pull request Feb 29, 2024
Merged
@projectgus

This comment was marked as outdated.

Sign in to view
@IhorNehrutsa

IhorNehrutsa commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

@dpgeorge

Ready

@dpgeorge

dpgeorge commented May 5, 2026

Copy link
Copy Markdown
Member

Thanks for rebasing.

But honestly, the increase in code size is quite substantial for a feature like this that's rarely used/requested so far.

@IhorNehrutsa
IhorNehrutsa force-pushed the signal_repr branch 2 times, most recently from 81987e0 to dda7fd4 Compare July 9, 2026 13:11
@IhorNehrutsa
IhorNehrutsa marked this pull request as draft July 10, 2026 06:11
@IhorNehrutsa
IhorNehrutsa force-pushed the signal_repr branch 6 times, most recently from 44951c7 to b744e98 Compare July 10, 2026 07:08
@IhorNehrutsa IhorNehrutsa changed the title extmod/machine_signal: Add signal_print() as repr() function. extmod/machine_signal: Add Signal print function. Jul 10, 2026
@IhorNehrutsa
IhorNehrutsa force-pushed the signal_repr branch 6 times, most recently from d334fd5 to 083cfa4 Compare July 10, 2026 09:21
@IhorNehrutsa
machine_signal: Add Signal print function.
500c453 
Test code is:
```
from machine import Pin, Signal
signal22= Signal(Pin(22, mode=Pin.OUT))
signal22
signal21= Signal(Pin(21, mode=Pin.IN), invert=True)
signal21
```
Ounput is:
```
Signal(Pin(22))
Signal(Pin(21), invert=True)
```
instead of
```
<Signal>
<Signal>
```

Signed-off-by: IhorNehrutsa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jimmo jimmo Awaiting requested review from jimmo

Assignees

No one assigned

Labels

extmod Relates to extmod/ directory in source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@IhorNehrutsa @projectgus @dpgeorge @jimmo