BusPirate / Bus_Pirate

Community driven firmware and hardware for Bus Pirate version 3 and 4
622 stars 131 forks source link

Revise the binary protocol #52

Open agatti opened 7 years ago

agatti commented 7 years ago

The binary protocol hasn't changed with the introduction of the BPv4, which is just fine but it means that the two extra AUX pins cannot be controlled or interacted with via said protocol.

Changing this would mean bump up the revision on the protocol identifier (when there is one) and add/modify commands in a slightly more sensible way than the organically-grown set that is currently available.

As usual, there are some pros and cons to consider:

Pros:

Cons:

This is not set to any milestone as I guess it'll take a bit of time to reach a consensus on how this should be carried out, as at the moment there is no clear way forward.

agatti commented 7 years ago

51 depends on this.

dspmind commented 7 years ago

I think the most problematic issue is making the existing code obsolete. However (as you mentioned), this is the consequence of not adapting the protocol / firmware version negotiations to accommodate, backward compatible changes in the existing design. According to me the earlier it is done, the better. But enough thoughts need to be put forth so that robust feature negotiation protocol is also included along with bidirectional version negotiation and adaption. This should permit, deprecation, backward compatibility with new features etc. reflected through the error codes returned by the binary protocol, and take care of making the new features without breaking existing codes.

Anyway Thanks for taking up this feature.

agatti commented 7 years ago

One possible option would be to find a binary command pattern that is currently not used in the protocol and use that to switch the BP to the new protocol. Of course, if the old protocol support is not compiled in then the BP would directly respond with different signatures, allowing new software to probe for the existence of the new protocol switch and act accordingly.

dspmind commented 7 years ago

This sounds like a nice idea. But we need to make sure that it will scale well for the future.

andrewd207 commented 7 years ago

I implemented this a while ago. It's been a couple of years but I will try to find the code. I made some changes that break avrdude for example. But it was easy to fix. If I remember, I made the mode switch command two bytes, allowing many modes. So bin0 became bin1 and then to bitbang I added a new mode bbio1 or something. It allowed setting all io pins with one byte. Setting power pins was a two byte command. Anyway it was a while ago and I might misremember some of the things.

andersm commented 7 years ago

If the binary protocol is being extended, I would really like the option of using hardware I2C in binary mode.

leonerd commented 5 years ago

I don't know if this is the right issue to discuss it, but I have quite the list of "design annoyances" with the binary IO protocol, that I would love to see addressed in a future version. Most of them are just missing features (e.g. ability to read the AUX pin(s) when in a serial IO mode), but some are proper design mistakes (e.g. the unescaped dumping of incoming payloads in UART mode).

A lot of these have come to light during the creation of https://metacpan.org/pod/Device::BusPirate

Should I expand on them here, or create new specific issues for them each, or what would be a good plan?

agatti commented 5 years ago

@leonerd feel free to expand on them here, I'll move things into their own issues before starting on v7.2 .

leonerd commented 5 years ago

@agatti OK, here goes:

In any of the binary IO modes, the following is missing:

There's spare command numbers for adding this, so it should be easy enough.

In all binary IO modes apart from raw bitbang mode, the following features are missing:

These are likely solvable by just implementing the existing commands defined in bitbang mode, though they'll need their numbers moving as the number ranges overlap with SPI or I²C bulk write. Perhaps at that point if a new protocol version is being made, just swap the numbers to a different range so they remain the same in all protocol modes.

leonerd commented 5 years ago

Additionally, in UART mode, receiving is basically impossible to use correctly. The only way to receive UART input is to turn on receive mode, which is acknowledged by a single byte (0x01) and then incoming data is copied raw, without any escaping mechanism, until it is disabled again. The act of disabling it also yields a 0x01 byte. But how would you distinguish that from a real incoming byte of 0x01?

I'd suggest instead a "packeted receive mode" with a new on/off command. In this receive mode, received packets come in chunks of perhaps 1 to 16 bytes, prefixed by a byte saying how many bytes will be in this packet:

0000010x - turn on/off packeted receive mode

Packets are sent from the Bus Pirate prefixed by a byte 0001xxxx where xxxx gives the number of bytes in the packet in the same offset-1 style as bulk write.

This would remove the ambiguity in the command responses.

leonerd commented 5 years ago

This one is purely a "would be really nice to have", but I've often wanted an edge-triggered interrupt input of some kind, to avoid busy-polling of the AUX pin just to find out if my slow sensor / etc.. is ready yet.

A new command pair could be added that applied to all binary protocol modes, of using the AUX pin (or a choice of them in the case of BPv4 hardware) to this purpose:

011000xx - read AUX pin as edge-triggered interrupt source
   xx = 00 - off
      = 01 - rising edge
      = 10 - falling edge
      = 11 - rising or falling

Interrupts can be reported as serial writes directly to the PC, as at this point the meaning of any inbound byte would be known (as in raw bitbang, SPI and I²C modes, the BP never talks unless in response to a command, and in UART mode the packeting ensures the first byte encodes the packet length), so some byte like 0011000x can be used - the x bit indicating rising or falling edge.

leonerd commented 5 years ago

Another missing feature: in raw bitbang mode, there's no open-drain option. Any output pins are always totem-pole. I wonder if this could be solved while also tidying up some of the differences between all the protocols.

Maybe in all modes, the top few code values could set up configuration options; e.g.

11110xyz - configure IO lines
    x = totempole(1) / opendrain(0)
    y = power on
    z = pullups on

Then in bitbang mode, use the pair

010xxxxx - configure IO pins
011xxxxx - write output pins

As it seems unlikely that users will often need to combine configuration (power/pullup/totempole) and output setting, as they currently do with 1xxxxxxx

Overall, I think some neateness could arise from trying to find common command values across all the modes, for setting things that truely apply across them (e.g. the hardware configuration), and also using the same numerical values for at least similar concepts (like the bulk write of 0001xxxx).

leonerd commented 5 years ago

@agatti

One possible option would be to find a binary command pattern that is currently not used in the protocol and use that to switch the BP to the new protocol. Of course, if the old protocol support is not compiled in then the BP would directly respond with different signatures, allowing new software to probe for the existence of the new protocol switch and act accordingly.

A quick test on my BPv3 running 3.6, shows that sending the byte 0x0E makes it respond NACK (0x00), so perhaps this could be used as a "switch version" command. I often find those work best if the requestor sends a min/max range:

00001110 - request protocol version upgrade
    The next two bytes are the minimum and maximum the caller is able to support.
    Bus Pirate should pick the maximum version within this range that it recognises.
    The response will be the command byte echoed back, followed by a single byte
    giving the chosen version, or a single NACK (0x00) if no suitable version is
    available.
leonerd commented 5 years ago

One other observation: In plain bitbang mode, the main "configure peripherals" and "set output pin values" commands each just respond a single byte, containing all the pin states. For similar reasons to the UART receive mode problem, this makes it harder to handle spurious input (e.g. of pin change interrupt ability). To fix this and in keeping with other ideas, these responses should be changed so their bitpattern looks a lot more like the command that sent them - i.e. having the topmost bit set.

In general having responses "look like" the commands that sent them, means that the same design shape is likely to work. Since the BP has to be able to work out what these bytes mean, pulling them apart as it goes, if the responses to them look similar then the PC can likely do similar to work those out too.

agatti commented 5 years ago

@leonerd Thank you for the report! I'm starting to clean up things a bit for the binary I/O code in preparation for this. I'll move entries to separate issues one by one as I work through them.

(I wish GitHub had sub-issues like Jira, so things could be arranged in a much cleaner way...)

leonerd commented 5 years ago

I've just thought of another one: bulk I²C data reads would be a little bit more efficient if there was a combined "read byte and send ACK/NACK" command. The values 0000100x are free for this purpose.

I wonder if perhaps we should start a document somewhere to combine the docs for all these new suggested features, and see if they all look right together and so on.

@agatti - slightly offtopic for the Bus Pirate itself, but I'm also working on a new piece of hardware to reimplement the binary IO parts of the Bus Pirate, while at the same time considering extending the protocol a bit with some new features (more IO pins with interrupt support, mostly) - I wonder if maybe email might be best to think on that one?

agatti commented 5 years ago

@leonerd the document could really help, I think I started to get something like that based on doxygen (check the 1-wire source code, for example) but that clearly did not scale and I sort of terminated the experiment.

For your custom hardware, sure, keep in mind that there are two other issues regarding interrupt-driven I/O for the BP (#51 and #95). My email is in every git commit of this repo, after all :)

leonerd commented 5 years ago

I've started off by just drawing out some tables of the existing protocol here:

https://docs.google.com/spreadsheets/d/1Yv9yEJ4ZIuHrKfkjfk5l-P2wZR1SIgENmGUI8a4Tn4M/edit#gid=0

My next plan is to work through these, add some "missing features" lists to each tab, and then write up a document suggesting a new version of protocol.

I'm trying to work out exactly how much to try to keep existing protocol numbers fixed - do you think it's worth that? On the controller side, a PC should be fine with entirely new numbers/protocol shape for a "version 2", but I wonder if it might help to keep things compact and small, to minimise the number of changes made, so the implementation can be shared between v1 and v2 as much as possible.

leonerd commented 5 years ago

I have written what I feel is a first-draft for a version 2 protocol here.

https://docs.google.com/document/d/15SawEMBskD7MhfgTShlbijmxylipp15KBu7T-ttqXL8/edit?usp=sharing

There are still a few TODOs in there - mostly just a matter of copying wording from v1 protocol documentation. A few of these themselves I'm not 100% clear about:

leonerd commented 5 years ago

Latest changes: I've added digital IO commands for ADC(P7) and DAC(P8) pins, to match AUX/CS/etc... Also added the pullup voltage configuration that BPv4 added.

leonerd commented 5 years ago

I've started implementing this now, both in a device (currently based on an ATtiny4313 microcontroller), and on the PC controller side (in a branch of my Device::BusPirate module). I have the basic bits of BBIO, SPI and I²C, though I haven't yet added the write-then-read ("WtR") ability of SPI or I²C.

Two further thoughts occur to me on this command:

To solve these, I think this should be fairly uncontentious to add a version of I²C WtR that doesn't send START/STOP. More difficult may be to handle the buffer size. My current thought is to add a special form of NACK, which tells the controller also what size of bytes the device can support.

For example; suppose the controller wanted to WtR a 258 byte block (2 bytes of address and 256 bytes of data). It would first send the WtR command header, requesting these sizes:

08 01 02 00 00

If the device accepted it, a regular ACK (01) would come back, then the device waits for those 258 bytes of data. But in my device's case, the maximum supported buffer size is only 128 bytes, so it would say so:

0E 00 80

This tells the controller it will have to drop the chunk size of writes. This now means it can't use a single WtR transaction, so it'll have to use the non-START/STOP version with explicit START and STOP commands:

02
09 00 80 {128 bytes of data}
09 00 80 {128 bytes of data}
09 00 02 {2 bytes of data}
03

Does this feel reasonable? It effectively allows the protocol to remove that 4096 byte limit, and lets controllers probe for how much data the device can buffer up in one go - allowing bigger hardware devices that could handle even larger buffers than that, as well as smaller ones.

leonerd commented 5 years ago

Actually, thinking further about it I don't think the full generalness of the SPI-like write-then-read is necessary for I²C. Given the way I²C transactions actually work on the wire, every complete I²C transaction is either going to be "start; write some bytes; read zero; stop", or "start; write one byte; read some bytes; stop". Perhaps a neater command structure for this would be wanted.

I think a more reasonable setup would be to have options that make these two types of I²C transaction possible, by perhaps having to split over multiple commands accounting for limited device buffer size:

Overall that's quite a lot of options, if we want to allow all possible combinations of bordering START and STOP conditions and ACK/NACK behaviour that make sense

Such commands could look like:

001000xy - I²C bulk transfer (write N bytes)
    x = prefix with START
    y = suffix with STOP
  Command is followed by 16bit big-endian encoding of N. Device will ACK/NACK/ETOOBIG at
  this stage.
  If ACKed, device now expects to receive N more bytes of data. An optional START is sent,
  then the bytes are written, then an optional STOP. Device will now return ACK if all OK,
  EI2CNACK if the slave NACKed any byte except the final one.

001001xy - I²C bulk transfer (read N bytes)
    x = ACK last byte read (if xx=1 or 2)
    y = suffix with STOP
  Command is followed by 16bit big-endian encoding of N. Device will ACK/NACK/ETOOBIG at
  this stage.
  If ACKed, the bytes are read, then an optional STOP. Device will now return ACK followed by
  the read data. No START condition is sent - this must be done using another command first
  to write the slave-addressing byte.

001010xy - I²C bulk transfer (write 1, read N bytes)
    x = ACK last byte read
    y = suffix with STOP
  Command is followed by 16bit big-endian encoding of N. Device will ACK/NACK/ETOOBIG at
  this stage.
  If ACKed, device now expects to receive 1 byte more data. A START condition is sent, then
  the byte written, then bytes read, then an optional STOP. Device will now return ACK if all
  OK, EI2CNACK if the slave NACKed the written byte.

Overall, these three commands are a lot more flexible than the existing "bulk write" (which cannot read), or "write-then-read" (which has buffer size and atomicity limitations) commands. They're also fairly efficient in terms of the number of bytes transferred over the control UART connection as compared the I²C bus - which might be important for 400kHz bus speed. I think a case could be made that these can entirely replace existing uses of the two existing commands.

leonerd commented 5 years ago

Another thought: The chip I'm using (ATtiny4313) has easy ability to enable an internal pullup resistor per pin. This would make it possible to easily add commands to set various IO pins into read-with-pullup state. Perhaps the various "read" commands should take a bitflag to set this?

Primarily I'm thinking of the INT ability I have on IO pin 6. It's intended for edge-triggered interrupt reporting, the main use-case being to simplify the setup when talking to SPI or I²C sensors or other chips that might raise interrupts. Often these chips have open-drain outputs, requiring a pull-up resistor. A read-with-pullup command would be useful here, though needs careful thought about how it would interact with other operations on the same pin - when does the pullup persist, and when is it cancelled?

leonerd commented 5 years ago

Couple of small additional thoughts: perhaps mode-switch should be a two-byte command, given how rare it is. An extra byte doesn't consume much more bandwidth, as compared the greater flexibility for having all those extra numbers for people to use.

Given as my device has 8 useable IO pins (all on the same PORTB of my MCU, as it happens), I'd like an 8-bit parallel IO mode so I can bit-fiddle all 8 lines atomically. This would be a sortof super version of BB mode. I can imagine that being fairly rare, as most other BP-like devices probably wouldn't have 8 programmable lines. So it makes sense that I can put it up somewhere in a high-numbered "local experimentation" range or something.

leonerd commented 5 years ago

@agatti Any overall thoughts on the direction of this v2 update? I'm slowly reaching a point of (local) stability here in terms of hardware and software, but before I set anything in concrete as "public" I'd like some idea of what you think of the thing as a whole.

coelner commented 5 years ago

currently avrdude is not working with the latest binary (U_1-05112018.hex) from here: http://dangerousprototypes.com/forum/viewtopic.php?f=28&t=8498&start=120#p67834 Does this issue relate to that or should I open a new issue?

USBEprom commented 5 years ago

@coelner

Thank you for reporting it! I believe you should open a new issue report where you explain in detail what exactly is the problem. Thank you.

leonerd commented 4 years ago

Hello all:

It's been a year or so on this and in that time I've been working on the protocol design, as well as developing some software and hardware to go along with it.

The protocol design doc (https://docs.google.com/document/d/15SawEMBskD7MhfgTShlbijmxylipp15KBu7T-ttqXL8/edit#heading=h.zi6qonpvo9ag) is nearing a state where the world can start looking at it. The most recent thoughts are that protocol switch needs to be a two-byte command, so it frees up some more space for more commands I am thinking of adding.

There are still some questions left on protocol design, that I would like to have some input from others on. I'll outline my hardware in another post, but in summary I am working on building a variation on the BPv3 which has its own different abilities, and I start to wonder whether the BusPirate binary IO protocol itself needs some way to indicate hardware abilities, or query it for capabilities.

In summary, it seems that what I have created here, as a protocol for talking to the BP or other similar hardware like my recreation, is more generic and flexible than the existing BPv1 binary IO protocol. It would be great to be able to take advantage of that flexibility to design and build a variety of cross-compatible hardware with different abilities, that can all use the same IO protocol.


Edit: I've now right-aligned ADC reads because that seemed a simple and uncontentious thing to do

leonerd commented 4 years ago

To give a background/rationale on my various protocol additions (some of which add abilities that the original BPv3 or BPv4 hardwares can't do), I am working on a new hardware design which embodies the original spirit but adds many new abilities.

In brief, my hardware is now based on the ATtiny1616, and has a few hardware differences from the BPv3 which are relevant to protocol design:

I don't yet have hardware for it, but I am planning a more flexible power switching design - allowing 5V, 3.3V (or maybe others?) on the primary VCC IO pin (P9). Also switchable digital IO voltage (at least between 5V and 3.3V) IO voltage. Additionally, add a voltage + current monitor to the VCC line.