yaq-project / thorlabs-apt-protocol

MIT License
8 stars 2 forks source link

Chan_ident handling #2

Open ksunden opened 2 years ago

ksunden commented 2 years ago

Migrated from https://gitlab.com/yaq/thorlabs-apt-protocol/-/issues/4

chan_ident is one of the more frustrating, inconsistent (and nearly omnipresent) field in the APT protocol messages.

for many (especially newer, it seems) devices, this is almost always just 1.

For some, such as the BSC203, it is 1 when talking to the daughter boards, but some commands like identify use chan ident in a message to the motherboard (this is perhaps the most frustrating inconsistency)

In fact, some of the messages (first noticed in the piezo section) are documented as it always being 1, though they maintain the bytes associated with the field.

Where it was documented as always 1 I opted to omit the field from the function signature (and fill with a hard coded 1, though I'm not convinced that was the correct call. would it be better to be consistent (which is probably why they kept the field)?

I have also desired having a default for chan_ident, but this is not strictly trivial, as I also do not want to reorder fields

One option is to make all fields other than dest, source, and chan_ident keyword only, which would allow setting defaults. (If I did this, I would probably also set default for source and dest to 0x1 and 0x50, respectively)

thus the signature for mot_set_homeparams would be:

def mot_set_homeparams(
    dest: int = 0x50,
    source: int = 0x1,
    chan_ident: int = 0x1,
    *,
    home_dir: int,
    limit_switch: int,
    home_velocity: int,
    offset_distance: int,
) -> bytes:

(I was going to use mov_absolute, but that has an alternate mode which does not require the position, so it actually is not that unusual as the only other param already has a default)

Such a thing is a bit unusual, but legal in python. It would make the "simple" calls (like home) easier (just mot_mov_home(), perhaps with a dest for rack systems), but the more complex ones much more verbose (which isn't necessarily bad, as it increases readability)

The other advantage is that it allows for setting default=None for fields that appear in some versions of the protocol but not others (in general, fields are added in a seemingly backwards compatible way, though I have not done rigorous comparisons. I believe in general you can zero pad fields which were not present in previous instances, and they are appended to the end.

On a related note, for the current intended usecase, source is always 0x1, so should that just be omitted in all functions?

I lean towards keep it for the reason that I want this library to mirror the protocol as 1-to-1 as possible.

ksunden commented 2 years ago

@untzag commented:

Personally I like keyword-only.