jankae / LibreCAL

4 port eCal module
GNU General Public License v3.0
64 stars 19 forks source link

SCPI API comments #6

Closed miek closed 1 year ago

miek commented 1 year ago

I've been reading through the SCPI API documentation and I had a few questions/comments about some of the implementation details, particularly with regards to matching the specification (IEEE 488.2) where possible.

In the introduction, it's noted that the commands are case sensitive to allow for more flexibility when naming coefficients. I didn't really understand the reasoning here as I couldn't see any examples of the coefficient name forming part of the command. I think it'd be reasonable to allow case insensitive commands, while still requiring case-sensitive parameters? It's just really convenient when typing SCPI commands on the fly to not have to worry about case.

The behaviour around remembering the position in the command tree is unusual to me. For all other instruments I've used, they don't require the leading colon and they do not remember position between lines. They only remember the position between multiple commands on the same line (separated by a semi-colon).

For example, this would not work:

TEMP:STABLE?
STABLE?

but this would:

TEMP:STABLE?; STABLE?

This isn't strictly part of the spec and is included for information only, but as far as I know it's the most common way to do it - I think it'd be best to match that to be least surprising.

One other minor thing is that the *IDN? response format is in the spec - it should return <manufacturer>,<model>,<serial number>,<firmware version>. I think it'd be worth complying with that so other tools can parse it reliably.

jankae commented 1 year ago

I think it'd be reasonable to allow case insensitive commands, while still requiring case-sensitive parameters?

Correct, and already implemented like that in the firmware. Looks like the SCPI API needs to be described a bit more precise there.

They only remember the position between multiple commands on the same line (separated by a semi-colon).

I just had another look at the spec and I think you are right. I misinterpreted that part and thought it should always remember the position (unless the new command starts with a colon). Probably an easy fix.

One other minor thing is that the *IDN? response format is in the spec - it should return ,,,.

Also correct, it is probably best to comply with the spec here. Not sure what I am going to use for manufacturer and model since there is only the name of the project ("LibreCAL") so far. But I'll come up with something.

jankae commented 1 year ago

This should be (mostly) fixed now. The only thing that is missing is the support for multiple commands per line like this

TEMP:STABLE?; STABLE?

At the moment, each command must be issued on a separate line. This wasn't possible before either but the documentation didn't mention it. I am not sure if it is worth to add this, does anyone actually use it? The implementation would be a bit tricky with the way the SCPI server is implemented.

miek commented 1 year ago

This should be (mostly) fixed now.

Awesome, thank you!

The only thing that is missing is the support for multiple commands per line [...] I am not sure if it is worth to add this, does anyone actually use it?

Yeah, I think that's fine. I don't think it gets used very much, and it only really matters for optimising performance which shouldn't matter here.