machine: add no-op io.Reader implementation to NullSerial #2593
Conversation
bgould
changed the title
bgould
force-pushed
the
serial-none-noop-read
branch
4 times, most recently
from
7114854 to
d5916cb
Compare
bgould
force-pushed
the
serial-none-noop-read
branch
from
d5916cb to
277b3f7
Compare
|
renamed the unexported |
|
|
||
| // Read is a no-op; always returns that zero bytes were read with no error | ||
| func (ns NullSerial) Read(p []byte) (n int, err error) { | ||
| return 0, nil |
There was a problem hiding this comment.
Just wondering, shouldn't it return io.EOF to show that there is nothing to read?
As per doc
// Implementations of Read are discouraged from returning a
// zero byte count with a nil error, except when len(p) == 0.
// Callers should treat a return of 0 and nil as indicating that
// nothing happened; in particular it does not indicate EOF.
The same as ReadByte() always returns an error
Lines 32 to 34 in 3458726
There was a problem hiding this comment.
Agreed, returning an error seems more correct to me.
There was a problem hiding this comment.
Do you think should return io.EOF or should it return errNoByte (the same as ReadByte)?
|
|
||
| // Read is a no-op; always returns that zero bytes were read with no error | ||
| func (ns NullSerial) Read(p []byte) (n int, err error) { | ||
| return 0, nil |
There was a problem hiding this comment.
Agreed, returning an error seems more correct to me.
| // type check to ensure NullSerial implements interfaces for drivers.UART | ||
| var ( | ||
| _ io.Reader = NullSerial{} | ||
| _ io.Writer = NullSerial{} |
| @@ -22,7 +22,7 @@ type irqCtrl struct { | |||
| } | |||
|
|
|||
| type ioBank0Type struct { | |||
| io [30]io | |||
| io [30]rpio | |||
There was a problem hiding this comment.
Importing the io package as part of the type check causes a compile error in the smoke test for RP2040 because it defines an internal type called io as well. The alternative would have been to import io with an alias in serial.go, but I thought it would be better and more appropriate to rename the type for the rp2040 to prevent this problem from happening again in the future.
When compiling with
-serial=usbor-serial=uart,machine.Serialimplementsdrivers.UART.Currently the same is not true for
-serial=none, becausemachine.NullSerialdoes not implementio.Readeras necessary: https://github.com/tinygo-org/drivers/blob/a15f2167cc7e46864d82e08904a3ddaa3e610d5e/uart.go#L8Adding a no-op Read() method so that NullSerial implements
drivers.UART