Skip to content

Conversation

@AaronJackson
Copy link
Contributor

It's my first applet so there's probably a few things which aren't great - please let me know, happy to fix.

some examples:

$ glasgow run gpib-command -V5 --address 10  --command '*IDN?'  --read-eoi
I: g.device.hardware: device already has bitstream ID 9aa1d2585196e6a514d0d0faf2fc5872
I: g.cli: running handler for applet 'gpib-command'
I: g.applet.interface.gpib_command: port(s) A, B voltage set to 5.0 V

TEKTRONIX,TDS 3014,0,CF:91.1CT FV:v2.11 TDS3GM:v1.00 TDS3FFT:v1.00 TDS3TRG:v1.00

$ glasgow run gpib-command -V5 --address 10  --command 'HARDC STAR'  --read-eoi > tds3014.eps
I: g.device.hardware: generating bitstream ID 9aa1d2585196e6a514d0d0faf2fc5872
I: g.cli: running handler for applet 'gpib-command'
I: g.applet.interface.gpib_command: port(s) A, B voltage set to 5.0 V

$ eps2png tds3014.eps tds3014.png 1000

tds3014

how the CLI is structured currently:

  • if no address is specified, it'll default to address 0
  • if no command is specified, it'll default to listening
  • if read-eoi is specified, it'll listen until EOI is asserted. some commands do not respond

@AaronJackson AaronJackson requested a review from whitequark as a code owner May 2, 2025 21:25
@AaronJackson AaronJackson marked this pull request as draft May 5, 2025 23:25
@AaronJackson AaronJackson changed the title applet.interface.gpib_command: new applet for GPIB wip: applet.interface.gpib_command: new applet for GPIB May 5, 2025
def add_build_arguments(cls, parser, access):
super().add_build_arguments(parser, access)

access.add_pin_set_argument(parser, "dio", width=range(0, 8), default=(0,1,2,3,15,14,13,12))
Copy link
Member

Choose a reason for hiding this comment

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

This assignment is a bit weird, is there a rationale?

Comment on lines 419 to 430
async def write(self, control, data=bytes([0])):
await self._device.set_pulls(self._args.port_spec, high={pin.number for pin in self.talk_pull_high})
await self._gpib.write(bytes([ x for b in data for x in (control, b) ]))

async def read(self, gpib, to_eoi=False):
await self._device.set_pulls(self._args.port_spec, high={pin.number for pin in self.listen_pull_high})

eoi = True
while eoi:
await self.write(0b0100_0000)
eoi = (await self._gpib.read(1)).tobytes()[0] & 2
yield (await self._gpib.read(1)).tobytes()
Copy link
Member

Choose a reason for hiding this comment

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

These functions should be in a GPIBInterface. This way, other applets could reuse this one, and glasgow repl gpib-command lets you use the applet from the command line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to work, although GPIBMessage is kind of necessary to avoid specifying random numbers from the repl, and that won't be in scope - is there a nicer way of doing this?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, can you rephrase with more detail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, sorry!

It would nice if the GPIBCommand and GPIBMessage enums were in scope when using from the repl, so that we can do this iface.write(GPIBMessage.Data, b"*IDN?"), but this will say GPIBMessage is not defined

Maybe I should instead add some extra methods to the interface to make it more friendly.

ah, just saw your other comment - Yep, it needs some additional methods to hide the complexities slightly.

It's important though that low level features are still accessible. While it'd be fine for an oscilloscope, I think it should also be extensible to the weirder stuff that runs on GPIB (e.g. floppy drives 👀 )

Copy link
Member

Choose a reason for hiding this comment

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

You can copy the implementation of GlasgowApplet.repl to your applet and customize it however you see fit; adding GPIBMessage to the locals dict would be a way to do what you want.

Re: exposing low-level features: that's fine, but I think they probably need a more polished API. We can leave that to a later point; I think once you write a few helper methods for simple use cases like scopes or multimeters, we'll be in a better place to flesh out that API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up adding all the GPIBMessage types are separate methods, but I'm happy to remove them and stick to a few basic ones if you think it's more appropriate :)

They are all quite simple at least 😁

await iface.write(GPIBMessage.Command, bytes([GPIBCommand.MTA.value | address]))
await iface.write(GPIBMessage.Data, bytes(command.encode("ascii")))
await iface.write(GPIBMessage.DataEOI, b'\n')
await iface.write(GPIBMessage.Command, bytes([GPIBCommand.UNT.value]))
Copy link
Member

Choose a reason for hiding this comment

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

I think this is probably factored wrong. Although your Interface does allow submitting commands in a more structured way than before (this is great!), the intent behind this split is to have the Interface take care of command sequencing and such.

For example, although I don't fully understand GPIB, I would expect the four write() calls above to be done as a part of a single function, something like await iface.sendto(address, data). That would hide the implementation details, like needing to OR stuff together, or the explicit setting of EOI.

Imagine you are writing a filesystem driver. Your API consumers probably don't want to care which blocks on disk will contain the data, nor do they want to explicitly provide this information to the driver. All they want is a call like read_file(filename, offset, length) or somesuch.

Sometimes it is useful to provide very low-level access to bus features, but in general I've avoided doing so in public methods, since it can be difficult to ensure that the applet can evolve without breaking its public interface if you expose too much. You can still provide such access as _private methods, and are encouraged to do so if it helps you factor the applet itself into nicer pieces. (Take a look at other in-tree applets for inspiration, like the JTAG probe or ARM7TDMI one.)

@AaronJackson
Copy link
Contributor Author

Re IRC chat about FSM: How it works

The FSM is divided into three parts: Control, Talk and Listen.


Control starts by sitting idle until a byte is received from out_fifo. The received byte is latched into l_control

  • tx - indicates that a second byte must be latched, this is l_data, and what should be written out to the bus if set.
  • eoi - whether to make the EOI line active. This is generally used to signify the last byte of a transmission, although there have been other uses for it.
  • ifc - whether to make the IFC line active. This is used to reset all devices on the bus, although the exact purpose may be different between manufacturers. Generally though, if an error has occur on the bus and a device won't respond, setting this for 150uS will reset their error register so the process can restart. Most of the more modern equipment doesn't seem to bother with this.
  • ren - whether to make the REN line active. This is "Remote ENable". Some early GPIB equipment would lock out the front panel when under remote control - that would happen automatically if you send a command, but asserting this line is a way to do that without sending a command.
  • reserv - this is unused and takes up one bit of space 👀
  • listen - if set, the FSM will move to Listen: Begin - this ignores any of the signalling lines (eoi, ifc, ren)
  • talk - if set, the FSM will move to Talk: Begin - this is where signally lines are set. if tx is also set, it'll output the byte latched in l_data.

The next state for "Control: Begin" is always "Control: Read data". If the listen bit was set, it will skip reading data and move onto parse, otherwise latch data, then move to parse.

"Control: Parse" determines what to do next based on the listen and talk bits - if both are asserted it'll move to "Control: Error" (something better needs to happen here). There is one other Control state, "Control: Acknowledge" which is used after talking, but I'll say more on that in a bit.


Talk immediately sets the signalling lines based on l_control. if tx was unset, it'll jump to "Control: Acknowledge", otherwise it'll begin the process of writing to the bus.

Let's talk about GPIB handshaking 😎 There are three handshaking lines: NRFD (Not Ready For Data), NDAC (Not Data Accepted) and DAV (Data Valid). While talking, the controller passively pulls these lines to 5v. Each device has the ability to assert these low (active) by tying them to ground. This enables the speed of the bus to be dictated by the slowest device on the bus.

During transmission, the controller will wait for NDAC to be asserted, the controller can then place data onto the data lines. The FSM uses a small timer to give some time for the data lines to stabilise - this isn't really necessary with short cables, and I haven't found an exact recommended time to wait, but given that GPIB can be wired in both star and bus configuration, or a mixture of the two, it seems like it's worth waiting at least a little.

Once the timer is zero, it'll check to make sure NRFD is not asserted, then assert the DAV line. It'll remain in this state until all devices have unasserted NRFD, and then NDAC. DAV can then be unasserted. Waiting for NRFD and NDAC separately is important, because we need to wait for the slowest device.

The Talk part of the FSM then jumps to "Control: Acknowledge" - Reminder, we can get to "Control: Acknowledge" without doing much "talking", i.e. if tx was unset, resulting in only some signalling lines changing.

"Control: Acknowledge" writes out a single byte GPIBMessage._Acknowledge - this lets the Python side wait for bus to be clear before jumping to (for example) a read, which changes the pull up resistor configuration. So, for each byte written, there's two bytes out and one byte in. (I've actually fixed this slightly by modifying the FSM to skip waiting for the data byte unless it's needed, I don't have my Yubikey to push to github 😭)


Listen is essentially the same but backwards. NDAC is asserted, NRFD is unasserted - we wait for DAV to be asserted and then move to "Listen: Management lines"

This currently only obtains the state of EOI (so we know if it's the last byte of data), and writes it to in_fifo. Then we move to "Listen: DIO lines" and write the data lines to in_fifo. From python this appears as: ask for data (GPIBMessage.Listen), wait for management (just EOI for now), wait for data. The Listen FSM will wait for unassert NRFD, wait for DAV and then unassert NDAC


😄

Things I'd like to add at some point:

  • Some kind of timeout for Talking and Listening, since it could just wait forever
  • The "Control: Error" will just stay there until reset, maybe that's not too bad if the GPIBInterface presents status nicely.

@whitequark whitequark added the waiting-on-review Status: Waiting for review label Jul 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-on-review Status: Waiting for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants