addressing feedback from @jepler #1
Conversation
|
@jepler Were you able to test this? |
adafruit_mcp2515/__init__.py
Outdated
| :param ~busio.SPI spi: The SPI bus used to communicate with the MCP2515 | ||
| :param ~digitalio.DigitalInOut cs_pin: SPI bus enable pin | ||
| :param int baudrate: The bit rate of the bus in Hz, using a 16Mhz crystal. All devices on the bus must agree on this value. Defaults to 250000. | ||
| :param bool loopback: Receive only packets sent from this device, and send only to this device. Requires that `silent` is also set to `False`, but only prevents transmission to other devices. Otherwise the send/receive behavior is normal. |
There was a problem hiding this comment.
please update (requires that silent be True)
| if loopback and not silent: | ||
| raise AttributeError("Loopback mode requires silent to be set") | ||
| if auto_restart: | ||
| raise AttributeError("`auto-restart` is not supported by hardware") |
There was a problem hiding this comment.
would it be useful to emulate this by restarting at any in_waiting(), receive() or send() call? is the checking expensive (I could believe that)
There was a problem hiding this comment.
I think it makes sense to defer implementation until its absence is raised as an issue
| # TODO: Timeout | ||
| tx_buff = self._get_tx_buffer() # info = addr. | ||
| if tx_buff is None: | ||
| raise RuntimeError("No transmit buffer available to send") |
There was a problem hiding this comment.
OK this does implement what the core docs say. It's not what core does. sigh. We can let some actual experience inform us as to what is preferable, and then implement it across the various chips, I don't want it to hold things back at this point.
There was a problem hiding this comment.
I agree that we should wait and see how things go.
| elif self.silent and not self.loopback: | ||
| raise AttributeError( | ||
| "Hardware does not support setting `matches` in when\ | ||
| `silent`==`True` and `loopback` == `False`" |
There was a problem hiding this comment.
I think they imagine that if you're in the monitoring mode that you want to "hear" everything?
heya @jepler, this should address your feedback. I also left some comments on the google doc.
This is going to fail because I hate RST and am tired of futzing with it