Dynamic control #63
Dynamic control #63
Conversation
This auto-fetches STM32 CUBE HAL libraries from github v1.8.4, builds
firmware and generate helper scripts:
* flasher - to flash the firmware
* debug - to run debugger in background and connect gdb to it
Simple process to use:
$ nix-build
$ ./result/flasher
$ ./result/debug
...
(gdb)
Change-Id: Ibd68ce0ef40f1feb5ef731dca99088f2ff8013ea
Before introducing any improvements fix trivial formating issue
For now shell is enabled by default. Connect **PB5** pin to ground to disable it.
Implements several macros for shorten aliases for some __attribute__, ARRAY_SIZE and BUILD_BUG
|
Hi Yuri, Thanks for your PR. I had a quick look at the code and yes, some things definitely seem useful right away. For instance, the preprocessor macros. I will review the PR thoroughly and let you know my thoughts. Please don't expect a quick review, I'm going to a vacation for two weeks and after that I'm going to be pretty busy with work for another week or two at least. My apologies for not being able to review faster. Best, |
|
Excellent. I abandoned my PR for an alternative approach, but would love to see this polished and merged. |
|
@r2axz no problem. As I wrote, there is already well known problems, which should be fixed first. For some of them I have no much knowledge to resolve quick (need to longread manuals first). Another problems I will fix soon - for example duplication code with withing cdc_shell.c. And there more minor ideas for improvement the code (starting from applying clang-format, till adding the functionality like gpio-notiffications). I will go through patches by myself and left my thoughts withing comments. I got used to gerrit-review with per-commit discussion and tried to split commits on complete functionality blocks, some of them are not really needed here (like nix build) but it is up to you. BTW Im going to one week vocation right after you =) |
|
@kzyapkov it's a pity you abandoned it. Fill free to participate in discussions here. |
| @@ -42,6 +46,26 @@ __noinline static void cdc_shell_write_string(const char *buf) { | |||
| cdc_shell_write(buf, strlen(buf)); | |||
| } | |||
|
|
|||
| __noinline static void cdc_shell_msg(const char *fmt, ...) { | |||
There was a problem hiding this comment.
How about remove statically allocated messages (like cdc_shell_err_uart_missing_arguments) and place readable text to the place, where it printed from, with additional information which may be useful for user. And add logging levels like error and info
Like instead
cdc_sell_write_string(cdc_shell_err_uart_invalid_uart)
use
cdc_shell_err("invalid UART number %d, use numbers 1, 2, 3 for it", port)
| #define cdc_shell_gpion_pin_ref_uart3 cdc_shell_gpion_pin_ref(7) | ||
| #define cdc_shell_gpion_pin_ref_uart cdc_shell_gpion_pin_ref_uart1 | ||
|
|
||
| #define cdc_shell_pin_extra_func(func, var, pred, name, ...) do { \ |
There was a problem hiding this comment.
Here is not clearly understandable meta-magic, should place extra comments for it and maybe move to external file?
There was a problem hiding this comment.
Added comments. Think about split cdc_shell.c file splitting on sub-systems like next:
cdc_shell/general.c
cdc_shell/uart.c
cdc_shell/gpio.c
cdc_shell/config.c
What does @r2axz think about it?
| @@ -394,6 +762,33 @@ static const cdc_shell_cmd_t cdc_shell_commands[] = { | |||
| "Example: \"uart 1 tx output od\" sets UART1 TX output type to open-drain\r\n" | |||
| "Example: \"uart 3 rts active high dcd active high pull down\" allows to set multiple parameters at once.", | |||
| }, | |||
| { | |||
There was a problem hiding this comment.
How about split cdc_shell.c for sub-modules (like uart, gpio, config, etc.) and format this table from compiler sections, to make this project easilly expandable.
ein-shved
force-pushed
the
dynamic-control
branch
from
7fa1aed
to
19d5735
Compare
I needed a device with two two-pinned uarts and several gpio and with control via usb. This project was close to my needs, but lucks of gpio controls and had extra functionality which blocks almost all pins.
I suggest the basic version of dynamic controlling the gpio pins via shell to allow user use extra pins as gpio. Current version of this PR is not suitable to release, because it was not properly tested, definitely will brake some generic functionality and not optimal and pretty in some places.
Additionally I used here some language features like macro-metaprogramming and format output which was not used previously for some reason. It may be found not suitable by maintainers.
But from now I can use it for my purpose and I do not know when I will make it perfect. So I made this PR to bring maintainers' attention so they may find something useful from it.
This PR adds new control to shell:
gpio.This command accepts the gpio name as first argument in view
[gpio_][pin_][p]<P><N>wheregpio_,pin_andpare optional prefixes,<P>is the name of port likea,borc.<N>is the number of pin.Examples:
pa1,gpio_pin_pa10,b3More on this, it accepts massive aliases:
all- apply command to all pinsfree- apply commands to all pins which havefreestatus (see later about statuses)occupied- apply commands to all pins which haveoccupiedstatusblocked- apply commands to all pins which haveblockedstatusuartN- apply commands to all pins which may be used by uartcontrol,config,shell- aliases forpb5led- alias forpb13The pin may be in one of 3 states:
blocked- the pin is used by critical functionality like USB, boot control, or serial debugoccupied- the pin is occupied by generic functionality like uart, led status or configuration contolfree- the pin is free to be modified from shell as generic io.The second argument may be:
show- show current status of pin like next:free- set pin's status to free. If one of tx or rx pin of uart goes to free state, the uart will be swithed off.occupy- set pin's status to occupied.get- print value of pinset [args]- configure pin behaviourup- make output pin updown- make output pin downThere are defaults for first two arguments:
showallSo command
gpio,gpio allandgpio all showare equal.