Skip to content
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

introduce SCPI_HelpQ with descriptions #135

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

MisterHW
Copy link
Contributor

@MisterHW MisterHW commented Feb 3, 2022

Human-readability is often a welcome aspect when developing software for SCPI devices, but also valuable when testing configurations or debugging issues in a lab or test setup. Command syntax and parameter ranges are traditionally only documented in manuals or programmer's guides, but often control software allows sending raw commands and performing queries.

To both help people implementing and using systems based on scpi-parser and to help projects stand the test of time, it seems valuable for the device to be able to list its supported commands, preferably with a short description exposing expected parameter types and ranges.

This PR provides

  • SCPI_HelpQ() callback for a "HELP?" pattern and a
  • USE_COMMAND_DESCRIPTIONS option which extends _scpi_command_t by a description string.

Data returned is formatted as an array of arbitrary block data. Newline characters are absorbed into the data of each block so the output is easily readable and can be parsed and displayed in control software (e.g. to provide a list of commands or syntax help).

Usage of the new features is demonstrated in the common example code.

image

The first column seen in the screenshot originates from the necessary block header information.

Memory Use

Backward compatibility is provided by setting USE_COMMAND_DESCRIPTIONS 0, not using SCPI_HelpQ() and if needed, by instructing the linker to drop unused sections (--gc-sections).
Even with USE_COMMAND_DESCRIPTIONS 0, the help command can provide value by accessing the pattern strings already in place.
When setting USE_COMMAND_DESCRIPTIONS 1 , _scpi_command_t is exended by a pointer (default: nullptr) to a null-terminated description string.

resolves #129

scpi-99 6.2.3.4 and 6.2.6 suggest returning multiple blocks is valid, so SCPI_ResultArbitraryBlockHeader() needs to insert a delimiter,
other changes are: accepting _GLIBCXX_HAVE_STDBOOL_H, testing for non-NULL data in writeData(), and some spelling fixes.
add SCPI_HelpQ callback , optional .description strings and provide conditional initializer macros dependent on config settings for USE_COMMAND_TAGS and USE_COMMAND_DESCRIPTIONS.
introduce search string to narrow down help output
…lt disabled

* Properly integrate command descriptions into examples. Preliminary descriptions added after consulting scpi-99 standard, as well as looking at the DMM and TEST handler source. Command paramater types hinted as <param> where appropriate. Return values or ranges given at the end of each description in round brackets.
* Default changed to USE_COMMAND_DESCRIPTIONS 0 to avoid unexpected impact on code side. Set to 1 to use.
* scpi_command_t defintion helper macros moves to types.h
* fixes some inaccuracies in scpi-def
* clean-up in utils.c
* add documentation to introduced functions
* correct CHAR_TO_LOWER
Allow makefile to be used directly for cross-compiling (e.g. when treated as a git submodule). This requires CC and AR to be passed for a particular toolchain, but CFLAGS must also specify CPU and FPU options -> introduce MODE variable. CFLAGS cannot be passed without need to override in the makefile, so a new variable is needed.

Cortex-M7 Example (linker: library and project use of VFP registers must match):
CC=arm-none-eabi-gcc AR=arm-none-eabi-ar MODE="-mcpu=cortex-m7 -mthumb -mfpu=fpv5-sp-d16 -mfloat-abi=hard"

(commit can be cherry-picked)
When compiled with USE_HELP_FILTER 1, HELP? accepts an optional search string -> HELP? command description extended to reflect input and output types.
Copy link
Owner

@j123b567 j123b567 left a comment

Choose a reason for hiding this comment

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

Thank you for your effort. Can you please rebase your branch to the current master? It now contains all the necessary changes to make this work. If you need to change formatting or correct typo, please provide different PR so it is easier to merge simple changes.

Can you please provide also PR for branch gh-pages so it is properly documented? Or at least a new small section to readme.md that this extension is available, how to use it and promote it that it can be helpful.

examples/common/scpi-def.c Show resolved Hide resolved

const scpi_command_t scpi_commands[] = {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you please provide alternative definition files for this functionality?

It is a generally useful but unusual extension. I would like to keep examples simple. Users can decide on their own if they would like to use help command or not and it is generally not needed. It will find its way, but I would like to keep the first impression simple.

Copy link
Owner

Choose a reason for hiding this comment

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

Alternatively, somehow divide the existing source (it is just an example anyway) so one part of the commands has help and the other does not so it is clear, that it is not mandatory to use and define help and tag.

Copy link
Contributor Author

@MisterHW MisterHW Jan 17, 2023

Choose a reason for hiding this comment

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

scpi-def.c and scpi-def.cpp are essentially identical. scpi-def.c could be stripped down to illustrate what a version with no help and no command tags could look like. One could additionally introduce

#define NO_CMD_DESC SCPI_CMD_DESC(NULL)

which would help being explicit about the generalized structure. With scpi-def being in common, offering definitions for all possible cases seems a bit excessive to me.

Retrospective Context:
Keep in mind that prior to this PR, the default was

#define USE_COMMAND_TAGS 1

and setting it to 0 would cause a type mismatch, as in scpi-def.c/.cpp there were extra (unconditional) , 0 in the scpi_commands[] items.

The documentation on HELP? might be the best place to elaborate on way to further strip down the definition and explain the implications. This was warrented through the existence of USE_COMMAND_TAGS, and in my opinion is underscored yet again through the introduction of USE_COMMAND_DESCRIPTIONS.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, scpi-def.cpp is identical to the scpi-def.c but all C specific features are removed so it can be compiled by the C++ compiler. Because this is C library, C++ file is just an example without real usage here.

Initially, examples were more about the communication interface and the command interface was the same. In the current perspective, there could be more standalone examples promoting separate features of the library, like enabling/disabling TAGs, and more importantly, introducing HELP? command.

Maybe, there can be a new example based on e.g. test-interactive to show, how HELP? command works and how to define all strings. It can have a completely different structure and its own commands definition.

examples/common/scpi-def.cpp Show resolved Hide resolved
examples/common/scpi-def.cpp Show resolved Hide resolved
libscpi/Makefile Outdated Show resolved Hide resolved
libscpi/inc/scpi/types.h Outdated Show resolved Hide resolved
libscpi/inc/scpi/types.h Show resolved Hide resolved
libscpi/src/utils_private.h Outdated Show resolved Hide resolved
libscpi/src/utils.c Outdated Show resolved Hide resolved
* @return Pointer to first match in s if found, otherwise `NULL`.
* @author Alvaro Lopez Ortega <alvaro@alobbs.com>
*/
char * strncasestrn (const char *s, size_t slen, const char *find, size_t findlen)
Copy link
Owner

Choose a reason for hiding this comment

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

Can you please provide unit test in test_scpi_utils.c

@coveralls
Copy link

coveralls commented Jan 17, 2023

Coverage Status

Coverage: 91.844% (-0.8%) from 92.594% when pulling a44b9aa on MisterHW:scpi_helpq_enhancement into e882505 on j123b567:master.

remove / comment currently unused code. Double-indirection helper definition moved to utils.h.
@MisterHW
Copy link
Contributor Author

Yeah well, C89 build fails because strncasecmp() was introduced later.

@j123b567
Copy link
Owner

Unfortunately, some manufacturers stick to C89 for their compilers (e.g. Microchip was using a prehistoric compiler for PIC32. Newer version is now available but hard to adopt because of frequent and incompatible changes in their core libraries)

@j123b567
Copy link
Owner

You can use use SCPIDEFINE_strncasecmp instead of strncasecmp. It is already provided by the library.

@MisterHW
Copy link
Contributor Author

I will get back to this eventually :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

introducing a help command
3 participants