Open mvladic opened 8 years ago
I know this limitation, but I don't know, how to handle this in pure c.
It is possible to have full table in PROGMEM and have RAM buffer for one item. Then while iterating over commad list, every command list item is coppied to RAM buffer and evaluated as usual. This can save some space, but I'm not able to do this.
If you can produce some working modification and way how to compile it and verify it (toolchain/ide/project), I will be happy. I can then add some ifdefs and merge it with main line.
I can also add ifdef
to int32_t tag;
in struct _scpi_command_t
in types.h
, so it will save some more space, if you don't use it.
Here are some more hints:
So findCommandHeader
will not do just
cmd = &context->cmdlist[i];
but some "deep copy" of cmdlist
item.
deep_copy_from_progmem(context->param_list.cmd, &context->cmdlist[i]);
context->param_list.cmd.pattern
must be preallocated with RAM buffer
Type of context->param_list.cmd
must be some new type to hold non constant pattern buffer.
Redefine some variables and fields to be in PROGMEM
(struct _scpi_command_t.pattern
, struct _scpi_t.cmdlist
, and scpi_commands[]
array itself)
I can't simulate it, so I will not do it blindfolded. If you give me some more hints, options for AVR-GCC, or so, I can do more.
You can now change USE_COMMAND_TAGS
to 0 to save some space, if you are not using tags in command list.
Thanks. I will try to do the rest by myself using your tips. I will post here about the progress.
I forked your project and created avr_progmem branch. By now, I created two commits:
1) support for 64K progmem
https://github.com/mvladic/scpi-parser/commit/d6ef5b156f39c9896bc46d21bba1bf6f9799308b
2) arduino example for 64K progmem
https://github.com/mvladic/scpi-parser/commit/dfebea1e8296ebcd185fe25847f06b1c2b352e90
It is slightly different to support only first 64K of program memory compared to the support of all memory (for example, AVR Mega 2560 has 256K of program memory). This is because AVR is using 16-bit pointers. Also, there is some overhead when you work with the full memory. So, my idea is first to add support only for 64K progmem, and I did it with these commits.
To use first 64K of progmem for command list you must set USE_64K_PROGMEM_FOR_CMD_LIST
to 1.
BTW I found and fixed small bug with SCPI_CMD_LIST_END
definition that is introduced with USE_COMMAND_TAGS
(check my first commit).
I tested the code on Arduino platform. You can download Arduino IDE and compile test-arduino example for yourself, but to be able to test it you will need one of the Arduino boards (I tested on Arduino Mega 2560).
To compile test-arduino under Arduino IDE you need to execute build-arduino-library.py first (more about this script below) than open test-arduino.ino sketch in Arduino IDE and press Verify button.
Unfortunately, there are some general quirks when you compile the code under Arduino IDE, especially when you use external libraries like scpi-parser. Because of that, I wrote the script called build-arduino-library.py to build Arduino compatible library from scpi-parser source code. It will build arduino library and copy it to the standard place for the arduino libraries (for example, on my Windows machine it is in C:\Users\Martin\Documents\Arduino\libraries) so Arduino sketches can use it. I'm running this script every time I change something in scpi-parser and in scpi_user_config.h located in test-arduino. There is no way you can set define variables in Arduino IDE so I need to place my define's in scpi_user_config.h and include it with the library. I also modify config.h on the fly when creating library for Arduino so that scpi_user_config.h is always included.
Here is screenshot of running the test-arduino with USE_64K_PROGMEM_FOR_CMD_LIST
set to 1:
It is using 2164 bytes of dynamic memory.
And here is when USE_64K_PROGMEM_FOR_CMD_LIST
is 0:
It is using 2706 bytes.
Nice! I will check it and play with it little bit.
I added support for the full program memory, here is the commit:
https://github.com/mvladic/scpi-parser/commit/533299d7f1b23bb9ade0e220be893bde342287d1
To support, not only first 64K, but full program memory there are some (minor) complications. Let me try to explain everything from the beginning.
TLDR; there is a question at the end :)
This is how SCPI commands are normally defined:
static const scpi_command_t scpi_commands[] = {
{"MEASure:VOLTage:DC?", DMM_MeasureVoltageDcQ, 0},
{"CONFigure:VOLTage:DC", DMM_ConfigureVoltageDc, 0},
// ...
};
scpi_t scpi_context = {
scpi_commands,
// ...
};
If we want scpi_commands array to be inside program memory, we can just add PROGMEM in scpi_commands declaration:
static const scpi_command_t scpi_commands[] PROGMEM = {
{"MEASure:VOLTage:DC?", DMM_MeasureVoltageDcQ, 0},
{"CONFigure:VOLTage:DC", DMM_ConfigureVoltageDc, 0},
// ...
};
scpi_t scpi_context = {
scpi_commands,
// ...
};
And this is how findCommandHeader()
could be implemented:
static scpi_bool_t findCommandHeader(scpi_t * context, const char * header, int len) {
int32_t i;
const char * pattern;
for (i = 0; (pattern = (const char *)pgm_read_word(&context->cmdlist[i].pattern)) != 0; ++i) {
if (matchCommand(pattern, header, len, NULL, 0, 0)) {
context->param_list.cmd_s.pattern = pattern;
context->param_list.cmd_s.callback = (scpi_command_callback_t)pgm_read_word(&context->cmdlist[i].callback);
context->param_list.cmd_s.tag = (int32_t)pgm_read_dword(&context->cmdlist[i].tag);
return TRUE;
}
}
return FALSE;
}
(Field context->param_list.cmd_s is added as temporary storage in dynamic memory for the found command.)
But, command pattern strings ("MEASure:VOLTage:DC?" and "CONFigure:VOLTage:DC") will not be stored in PROGMEM. To store pattern strings in program memory we must add PROGMEM specification for those strings too:
static const char DMM_MeasureVoltageDcQ_pattern[] PROGMEM = "MEASure:VOLTage:DC?";
static const char DMM_ConfigureVoltageDc_pattern[] PROGMEM = "CONFigure:VOLTage:DC";
static const scpi_command_t scpi_commands[] PROGMEM = {
{DMM_MeasureVoltageDcQ_pattern, DMM_MeasureVoltageDcQ, 0},
{DMM_MeasureVoltageDcQ_pattern, DMM_ConfigureVoltageDc, 0},
// ...
};
scpi_t scpi_context = {
scpi_commands,
// ...
};
And here is how findCommandHeader()
is implemented for this case:
static scpi_bool_t findCommandHeader(scpi_t * context, const char * header, int len) {
int32_t i;
PGM_P pattern;
for (i = 0; (pattern = (PGM_P)pgm_read_word(&context->cmdlist[i].pattern)) != 0; ++i) {
strncpy_P(context->param_list.cmd_pattern_s, pattern, SCPI_MAX_CMD_PATTERN_SIZE);
context->param_list.cmd_pattern_s[SCPI_MAX_CMD_PATTERN_SIZE] = '\0';
if (matchCommand(context->param_list.cmd_pattern_s, header, len, NULL, 0, 0)) {
context->param_list.cmd_s.callback = (scpi_command_callback_t)pgm_read_word(&context->cmdlist[i].callback);
context->param_list.cmd_s.tag = (int32_t)pgm_read_dword(&context->cmdlist[i].tag);
return TRUE;
}
}
return FALSE;
}
(Field context->param_list.cmd_pattern_s is added as temporary storage in dynamic memory for the found command pattern.)
Now, both scpi_commands array and all the patterns strings are stored in progmem. But, pointers are 16-bit so you can only address first 64K.
There is a way to get 32-bit (actually 24-bit) address of variable stored in program memory by using function pgm_get_far_address()
. But, this function can only be called in runtime, not in compile time, so it is not possible to build scpi_commands like above.
We can have a partial solution where scpi_commands array can be anywhere in the program memory, but pattern strings can only be inside first 64K (please read the code comments for the explanations):
// we need to change cmdlist field declaration in scpi_t
struct _scpi_t {
#if USE_FULL_PROGMEM_FOR_CMD_LIST
uint_farptr_t cmdlist;
#else
const scpi_command_t * cmdlist;
#endif
// ...
};
static const char DMM_MeasureVoltageDcQ_pattern[] PROGMEM = "MEASure:VOLTage:DC?";
static const char DMM_ConfigureVoltageDc_pattern[] PROGMEM = "CONFigure:VOLTage:DC";
static const scpi_command_t scpi_commands[] PROGMEM = {
{DMM_MeasureVoltageDcQ_pattern, DMM_MeasureVoltageDcQ, 0},
{DMM_MeasureVoltageDcQ_pattern, DMM_ConfigureVoltageDc, 0},
// ...
};
scpi_t scpi_context = {
0, // we can't get 24-bit address of scpi_commands during compilation
// ...
};
int main() {
// ...
// we can call pgm_get_far_address only in the runtime inside same function,
// here is called inside main function
scpi_context.cmdlist = pgm_get_far_address(scpi_commands);
// ...
}
And here is how findCommandHeader()
could be implemented for this case:
static scpi_bool_t findCommandHeader(scpi_t * context, const char * header, int len) {
int32_t i;
uint_farptr_t p_cmd = context->cmdlist;
PGM_P pattern;
for (i = 0;
(pattern = (PGM_P)pgm_read_word_far(p_cmd + offsetof(scpi_command_t, pattern))) != 0;
++i, p_cmd += sizeof(scpi_command_t))
{
strncpy_P(context->param_list.cmd_pattern_s, pattern, SCPI_MAX_CMD_PATTERN_SIZE);
context->param_list.cmd_pattern_s[SCPI_MAX_CMD_PATTERN_SIZE] = '\0';
if (matchCommand(context->param_list.cmd_pattern_s, header, len, NULL, 0, 0)) {
context->param_list.cmd_s.callback = (scpi_command_callback_t)pgm_read_word_far(p_cmd + offsetof(scpi_command_t, callback));
context->param_list.cmd_s.tag = (int32_t)pgm_read_dword_far(p_cmd + offsetof(scpi_command_t, tag));
return TRUE;
}
}
return FALSE;
}
To put everything anywhere in program memory space we need to do something like this (again, please read the code comments for the explanations):
// we need to add a new field in scpi_t
struct _scpi_t {
#if USE_FULL_PROGMEM_FOR_CMD_LIST
uint_farptr_t cmdlist;
uint_farptr_t cmdpatterns;
#else
const scpi_command_t * cmdlist;
#endif
// ...
};
// define all command pattern strings as single string like this:
static const char scpi_command_patterns[] PROGMEM =
"MEASure:VOLTage:DC?"
"CONFigure:VOLTage:DC";
// in scpi_commands array, pattern field of scpi_command_t is used to store
// not the pattern itself but the size of pattern string
static const scpi_command_t scpi_commands[] PROGMEM = {
{(const char *)(sizeof("MEASure:VOLTage:DC?") - 1), DMM_MeasureVoltageDcQ},
{(const char *)(sizeof("CONFigure:VOLTage:DC") - 1), DMM_ConfigureVoltageDc},
// ...
SCPI_CMD_LIST_END
};
scpi_t scpi_context = {
0, // we can't get 24-bit address of scpi_commands during compilation
// ...
};
int main() {
// ...
// in runtime we can set cmdlist and cmdpatterns far addresses:
scpi_context.cmdlist = pgm_get_far_address(scpi_commands);
scpi_context.cmdpatterns = pgm_get_far_address(scpi_command_patterns);
// ...
}
And here is the implementation findCommandHeader()
for this case:
static scpi_bool_t findCommandHeader(scpi_t * context, const char * header, int len) {
int32_t i;
uint_farptr_t p_cmd = context->cmdlist;
uint_farptr_t p_pattern = context->cmdpatterns;
uint16_t pattern_length;
for (i = 0;
(pattern_length = pgm_read_word_far(p_cmd + offsetof(scpi_command_t, pattern))) != 0;
++i, p_cmd += sizeof(scpi_command_t), p_pattern += pattern_length)
{
strncpy_PF(context->param_list.cmd_pattern_s, p_pattern, pattern_length);
context->param_list.cmd_pattern_s[pattern_length] = '\0';
if (matchCommand(context->param_list.cmd_pattern_s, header, len, NULL, 0, 0)) {
context->param_list.cmd_s.callback = (scpi_command_callback_t)pgm_read_word_far(p_cmd + offsetof(scpi_command_t, callback));
context->param_list.cmd_s.tag = (int32_t)pgm_read_dword_far(p_cmd + offsetof(scpi_command_t, tag));
return TRUE;
}
}
return FALSE;
}
Now, the question is do you want to add all these changes in the scpi parser? I want blame you if you find it too specific (or even ugly) to add it.
Another possible solution, which will solve my problem, is to give a user of the library a way to implemenent his own version of findCommandHeader()
function. This could be done like this:
struct _scpi_interface_t {
// ...
scpi_bool_t (*findCommandHeader)(scpi_t * context, const char * header, int len);
};
static scpi_bool_t findCommandHeader(scpi_t * context, const char * header, int len) {
if (context->interface->findCommandHeader) {
return context->interface->findCommandHeader(context, header, len);
}
// ...
}
Of course, matchCommand()
function must be part of documented API, so user implementation of findCommandHeader()
can use it.
With this solution, you could still include test-arduino in the scpi parser examples with the custom implementation of findCommandHeader()
which works with command list stored in program memory.
Wow, it is very interesting, but I have currently not much time to incorporate so big change. I have just few things.
Are there any drawbacks to leave it under 64k? Or the linker is not intelligent much and can place progmem section over this boundary?
Is it possible to add PROGMEM
keyword to type deffinition? So you can do somethink like this?
struct _scpi_command_progmem_t {
const PROGMEM char * pattern;
scpi_command_callback_t callback;
#if USE_COMMAND_TAGS
int32_t tag;
#endif /* USE_COMMAND_TAGS */
};
typedef struct _scpi_command_progmem_t PROGMEM scpi_command_progmem_t;
So you can leave command list as it was in original source but with scpi_command_progmem_t
type?
I don't like dividing command list to two more arrays, because this will cause big problems. If this is really needed, you can read somethink about x-macro https://en.wikipedia.org/wiki/X_Macro and automate generation of all lists so it is not possible to make a mistake.
I'm using X-macro in this file libscpi/inc/scpi/error.h
, after that, enum
is generated using one macro and SCPI_ErrorTranslate
is generated using second macro in error.c
, but there is only one LIST_OF_ERRORS
.
Are there any drawbacks to leave it under 64k? Or the linker is not intelligent much and can place progmem section over this boundary?
Linker will use program memory under 64k until we full this region. If our static data is bigger than 64K then it will use the memory above 64K (and pointers will overflow without warning). In my case, I need to store lots of static data like fonts, bitmaps, etc... so it is possible that at the end I will need more than 64K. Maybe I could put SCPI commands definition under 64K and other data above (still, I need to learn how to do that).
Is it possible to add PROGMEM keyword to type deffinition?
No, this is not possible. You can only place global const variables in PROGMEM section and you need to initialize those variables at the compile time.
I don't like dividing command list to two more arrays, because this will cause big problems. If this is really needed, you can read somethink about x-macro https://en.wikipedia.org/wiki/X_Macro and automate generation of all lists so it is not possible to make a mistake.
Actually, in my example test-arduino I was using something like x-macro (although I didn't know it is called x-macro). If you take a look on this file:
https://github.com/mvladic/scpi-parser/blob/avr_progmem/examples/test-arduino/scpi-def.cpp.
you will find the following (for the sake of brevity I removed some parts):
#define SCPI_COMMANDS \
SCPI_COMMAND("*CLS", SCPI_CoreCls) \
SCPI_COMMAND("*ESE", SCPI_CoreEse) \
SCPI_COMMAND("*ESE?", SCPI_CoreEseQ) \
SCPI_COMMAND("*ESR?", SCPI_CoreEsrQ) \
...
#define SCPI_COMMAND(P, C) static const char C ## _pattern[] PROGMEM = P;
SCPI_COMMANDS
#define SCPI_COMMAND(P, C) {C ## _pattern, C},
static const scpi_command_t scpi_commands[] PROGMEM = {
SCPI_COMMANDS
SCPI_CMD_LIST_END
};
So, you only need to define one array. (I forgot to #undef
after first SCPI_COMMAND
definition so you will get a warning).
I just learned that you actually don't need this trick. You can declare commands array like before by using PSTR macro:
static const scpi_command_t scpi_commands[] PROGMEM = {
{PSTR("MEASure:VOLTage:DC?"), DMM_MeasureVoltageDcQ, 0},
{PSTR("CONFigure:VOLTage:DC"), DMM_ConfigureVoltageDc, 0},
// ...
};
Now, both scpi_commands array and command patterns will be stored in program memory.
Still, for the full memory access we will need x-macro.
I'm using X-macro in this file libscpi/inc/scpi/error.h, after that, enum is generated using one macro and SCPI_ErrorTranslate is generated using second macro in error.c, but there is only one LIST_OF_ERRORS.
BTW I would like to store error messages in program memory also ;) What do you think of idea that user could provide implementation of SCPI_ErrorTranslate()
using scpi_interface_t
like I proposed for findCommandHeader()
in previous post?
Sorry, I didn't look into the source. I just read the comment. So it is much nicer with this.
I have played little bit with Arduino IDE and it seems, that your previous solution is the only one saving some RAM.
SCPI_ErrorTranslate
should be much easier to convert but still needs intermediate RAM buffer for one string.
So I will think about some API for ErrorTranslate
and findCommandHeader
so we can put them into separate file specific for AVR and keep rest of sources clean.
How much of trouble is to support SCPI commands list in PROGMEM on AVR? Usually, there is not much SRAM on these microcontrollers (in my case 8K) and my SCPI commands list is taking a lots of it and soon it will became unsustainable :(
BTW We are using your SCPI parser on this hobby project:
http://www.eevblog.com/forum/projects/diy-programmable-dual-channel-bench-psu-0-50v3a/
Once more, thanks for your excellent parser!