Skip to content

extmod/machine_signal: Add signal_print() as repr() function.#12291

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

extmod/machine_signal: Add signal_print() as repr() function.#12291
IhorNehrutsa wants to merge 1 commit into
micropython:masterfrom
IhorNehrutsa:signal_repr

Conversation

@IhorNehrutsa
Copy link
Copy Markdown
Contributor

@IhorNehrutsa IhorNehrutsa commented Aug 23, 2023

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
Copy link
Copy Markdown

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

Code size report:

Reference:  samd/machine_adc: Fix the configuration with averaging enabled. [21b3a51]
Comparison: machine_signal: Add signal_print as repr function. [merge of e63c0c8]
  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:  +136 +0.016% standard
      stm32:   +64 +0.016% PYBV10
      esp32:   +60 +0.003% ESP32_GENERIC[incl +16(data)]
     mimxrt:   +64 +0.017% TEENSY40
        rp2:   +64 +0.007% RPI_PICO_W
       samd:   +68 +0.025% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:   +71 +0.016% VIRT_RV32

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 23, 2023 •
edited
Loading

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.45%. Comparing base (9f396bb) to head (e63c0c8).
⚠️ Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
extmod/machine_signal.c 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #12291      +/-   ##
==========================================
- Coverage   98.46%   98.45%   -0.01%     
==========================================
  Files         176      176              
  Lines       22811    22813       +2     
==========================================
  Hits        22460    22460              
- Misses        351      353       +2

☔ View full report in Codecov by Sentry.
📢 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
Copy link
Copy Markdown
Contributor Author

IhorNehrutsa commented Aug 25, 2023

@jimmo
Could you give your opinion about this PR?

jimmo
jimmo reviewed Sep 1, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@jimmo jimmo left a comment

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
Copy link
Copy Markdown
Contributor Author

IhorNehrutsa commented May 4, 2026

@dpgeorge

Ready

@dpgeorge
Copy link
Copy Markdown
Member

dpgeorge commented May 5, 2026

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 @jimmo
machine_signal: Add signal_print as repr function.
e63c0c8 
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]>
Co-Authored-By: Jim Mussared <[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