-
Notifications
You must be signed in to change notification settings - Fork 228
wip: applet.interface.gpib_command: new applet for GPIB #796
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| 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)) |
There was a problem hiding this comment.
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?
| 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() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 👀 )
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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])) |
There was a problem hiding this comment.
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.)
|
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
The next state for "Control: Begin" is always "Control: Read data". If the "Control: Parse" determines what to do next based on the Talk immediately sets the signalling lines based on 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 "Control: Acknowledge" writes out a single byte 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 😄 Things I'd like to add at some point:
|
4366b88 to
dab4f76
Compare
not quite sure how to handle GPIBMessage not being in scope from the repl
ff5dd86 to
3527704
Compare
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:
how the CLI is structured currently: