To get more familiar with the code base I tried to implement a small feature that i needed when experimenting with a LORA SX1280 xciever: reading and executing macros from a file.
I’ve implemented a quick (and dirty…) proof of concept that is working enough to start discussing about it
Please note that everything that follows is just a simple proposal and is open to comments and critics (== please do!)
File format for now is really simple. I did not use json for two reasons: comments are not possible and simplicity for now. But maybe it could be better to use json to be aligned with the other files format.
The syntax to use macros from file is a slight modification of the standard macro syntax with the : used as prefix.
The command (:) lists all macro files (*.mcr) on the current directory.
Note: to do this I’ve added a bool disk_ls(const char *location, const char *ext) function which is similar to disk_ls_handler but with optional filtering by extension. This function could be useful for other usages maybe.
To use a macro from file first you set the current macro file name with the command (:<file_name>)
Then you can list all the macros in that file with the command (:0)
To exec a macro you simply use the command (:<MACRO_ID>)
The code is still really dirty, but if someone want to review or test it I can link here the branch from my repo.
The part that I like less is how I implemented the code to execute the macro: due to the tightly coupling of the syntax parser and the command line code (and my still basic understanding of the code ) the only solution that I came up with was to inject the macro to the command line like an user would have done, i.e. clearing the current cmd line and adding one character I time from the macro.
Probably there is a better way to programmatically execute bus syntax.
Next I’m working on allowing to save/add a macro to an existing file from command line.
All comments/suggestions/critics would be really welcome and appreciated
Wow, I really like this. When I moved all the old mode macros (I2C scan, etc) to commands, I made a note to add user defined macros instead.
I’m working on something similar too with a tutorial command. Tutorials can have comments # or // that will be printed to the user terminal. Other lines are executed as commands or syntax.
I am also pondering how to inject things. I want to support both syntax and commands, so will probably inject into the queue like you’re doing.
If your macros will only contain syntax (no commands, just bus stuff) then I can probably tease apart the queue and the syntax processor in ui_process.c.
I’d love to look over the code and get this feature added.
macro load sx1280.mcr
What do you think of adding it a macro command instead of loading in the macro directly? You can find an example command in /commands/global/dummy.c. Then it can use the standardized help system, and the load stuff can be separated cleanly from the processing loops. It could also print the (:0) menu when the macro is loaded.
Also, macros will probably be mode specific. The active macro file could be saved as a setting in the mode configuration files and set automatically when the mode is entered.
I would also suggest getting rid of the : because I’ve moved all the old macros to commands. It will take some cleanup, I realize. 1. remove all old macro functions from the mode files and 2. mode structs. 3. a centralized function to deal with macros, instead of mode specific ones. I’d be happy to look at your code and work on this with you. Maybe I should give you git access so you can start a branch?
I’d love to look over the code and get this feature added.
Here is a merge diff:
And this is the complete branch:
What do you think of adding it a macro command instead of loading in the macro directly?
The active macro file could be saved as a setting in the mode configuration files and set automatically when the mode is entered.
I would also suggest getting rid of the :
All really good ideas, I’ll try to implement those suggestions as soon as I’ve time. Thanks a lot for taking the time to give me feedback
Maybe I should give you git access so you can start a branch?
Thanks, really appreciated but in this period I’ve really little spare time, I don’t know if it’s worth giving me git access, at least not until I’m a little more comfortable with the code
Regarding the code that does the macro injections is basically this:
char c;
char *m = "{ d:125 0xc0 }"; // For example
while (cmdln_try_remove(&c)) ; // Remove all stale input
while (*m && cmdln_try_add(m)) // 'type' macro
m++;
cmdln_try_add('\0');
return ui_process_syntax();
I think what is needed to remove this abomination is to decouple the syntax process from the command line management, so maybe like adding a function to programmatically send a command/macro to be processed (and also add that to the commandline history ?)
One last question: for bug/fixes/code add like the one we are discussing here, is this a good approach for you or it would be better to open a new github issue and track everithing there ? I’m asking because I know it’s difficult/time consuming to follow several different sources and I want to let you wast as little time as possible.
Here is always the best to communicate. I’m plugged into the forum 12 hours a day and we have the little live chat thing for quick conversations. Stuff on github lands in an inbox that I don’t check often.
I looked through the diff. It makes sense. I think I can use some of the newer parsing stuff to simplify implementation a bit.
How attached are you to ()? Command and syntax processing is somewhat sane at this point, but I’m just starting on macros. Having the macro handling in ui_process.c with it’s own parsing stuff feels a bit untidy.
macro load <file>
macro 0
macro 1
macro 2 -h
If macro becomes a command it can take advantage of the existing command line parsing utilities and help system. load the macro file, 0 to list macros, 1 to do the macro, add -h for macro specific help from the file.
It doesn’t feel as neat and fast as (1), so I’m hesitant.
I started a new branch called macro. There is a new macro command in command/global/, I moved the business bits from your diff there. I will get it working with the commands system and then clean out the old macro system.
List available files. The disk_ls with extension filter - maybe that can be integrated with the existing ls command?
That exactly what I tought, we could simplify code. Maybe also add an argument with flags like: show only files or dirs, show summary, show size or not, etc. So it could work like the current ls but also display more succinct information for case like the macro list. I’ll work on this in the next days
I’m working on the tutorial command. This function advances the command line history pointer. In the tutorial command it keeps all the tutorial steps in the history. I think the same would work for macros.
char c;
while (cmdln_try_remove(&c)) ;
instead of this, which clears out the current position. This way the up/down arrow keys can be used to repeat/flip through macro history.
I think this should also fix an issue in the powersupply command.
It’d be nice, backwards compatible and from what I gathered from the code easy to implement string ids instead of integers.
instead of macro 1 we could do macro get_status
thoughts?
Comment from github. I can implement this. Any thoughts?
A) I think that using IDs instead of integers is a good idea, as already said it’s backward compatible and will be more flexible
I’ll try to find time to implement it this we.
B) Loading macro automatically on startup i can think about a couple of different implementations, not sure what will be best:
using different default macro files for different modes, i.e. a file named spi.mcr, another named i2c.mcr, etc, which are automatically loaded when changing mode (if they exist)
or we can add a macro command, like macro -d (Default) to set current loaded macro file name as default to be loaded (globally or for that mode only?)
In this case the setting will be saved to a config file.
C) About sourcing multiple macro files at once this is a bit more tricky: the help is a bit misleading, because we are not loading a file, but simply setting the current macro file name. When a macro is executed the file is parsed line by line until the ID is found and then the macro executed.
To allow multiple sourcing we will need to really load and merge the macro files into memory. It could be done but I think we need to discuss it a little bit more (and also implement A) and decide on B) before).
But those are all nice suggestions (EDIT: I mean the dzervas suggestions)
The old BP3 had optional firmware where they supported the BASIC language, which allowed for flow control, loops, variables, and editing. You simply added a line number to the beginning of a command and it stored it in memory. So you could edit and save “programs” - if you get rid of the colon, then this is no longer an option. I think we should consider how someone edits and saves a complex script interactively. It’s not like micropython and Thonny, where you can edit on the host and then run on the BP. There’s danger of file inconsistencies.
) the only solution that I came up with was to inject the macro to the command line like an user would have done, i.e. clearing the current cmd line and adding one character I time from the macro.
