Improvements for cli behaviour #76

jupe · 2018-07-17T16:19:02Z

Status

READY

Migrations

NO

Description

Several improvements:

optional disable man pages - to save memory
variable can be also integer
store previous retcode to ? -variable
store prompt as PS1 -variable (possible to change using `set´ -cmd)
api to request screen size: cmd_request_screen_size(void) -> generates variables: COLUMNS, LINES
true and false -commands
ctrl+w support (remove previous word)
escape character parsing (IOTSYSTOOL-131)
Improve test coverage and added valgrind mem leak test

NOTE: known issues

args like "ab""cd" cause two arguments: ["ab", "cd"]
"~weird" escaped arg inputs like "\\\\"\" causes weird results: "\\\\"\"

Todos

Tests
Documentation

* add option to ignore man pages by flag CMDLINE_INCLUDE_MAN=<bool> * add true and false commands and tests for available operators and, or, semicolon * add retcode tests * cleaning comments

* rename define CMDLINE_INCLUDE_MAN to MBED_CMDLINE_INCLUDE_MAN * remove underscore macro - use it always * add warning trace if max arguments reached * clean up some compiler warnings

* allow to store also int values for variables * fixes alt+left behaviour * add "?" variable for last retcode * add function for request screen size

* add escaping support * prompt and retcode format is now as variable

jupe · 2018-07-20T05:16:21Z

#77 need to to be fixed first to get this PR green

test/Test.cpp

 }
-// // alias test
-// extern void replace_alias(const char *str, const char *old_str, const char *new_str);
-// TEST(cli, cmd_alias_1)


jupe · 2018-07-20T08:11:09Z

test/Test.cpp

+    ARRAY_CMP("\r\nvariables:\r\n"
+              "PS1='/>'\r\n"
+              "?=-1\r\n"
+              "\r\x1b[2K/> \x1b[1D", buf);


replace with macros

jupe · 2018-07-20T08:11:11Z

test/Test.cpp

+    ARRAY_CMP("\r\nvariables:\r\n"
+              "PS1='/>'\r\n"
+              "?=0\r\n"
+              "\r\x1b[2K/> \x1b[1D", buf);


replace with macros

source/ns_cmdline.c

 #define TRACE_GROUP "cmdL"

+#ifndef MBED_CMDLINE_BOOT_MESSAGE
+#define MBED_CMDLINE_BOOT_MESSAGE "ARM Ltd\r\n"


source/ns_cmdline.c

 #endif
 //include manuals or not (save memory a little when not include)
-#define INCLUDE_MAN
+#ifndef MBED_CMDLINE_INCLUDE_MAN


YifeiZuo01

some more: replace with macros
https://github.com/ARMmbed/mbed-client-cli/blob/135a301ceca2203937d8cbf40b40abede2d9168b/test/Test.cpp?utf8=%E2%9C%93#L662-L666

YifeiZuo01 · 2018-07-20T08:24:46Z

test/Test.cpp

    ARRAY_CMP("\r\nalias:\r\n"
-              "_                 'alias'\r\n"
+              "_                 'alias foo'\r\n"
              "\r\x1b[2K/> \x1b[1D", buf);


replace with macros

YifeiZuo01 · 2018-07-20T08:24:52Z

test/Test.cpp

              "foo               'bar'\r\n"
-              "_                 'alias'\r\n"
+              "_                 'alias foo bar'\r\n"
              "\r\x1b[2K/> \x1b[1D", buf);


replace with macros

YifeiZuo01

LGTM.

mikter · 2018-08-23T06:22:07Z

ok for me.
when using command line and pasting long command strings that goes past the screen used to cause wrong behaviour. is that handled in here or is it a known issue after this?

tommikas · 2018-08-23T06:38:43Z

If the limit is the line going past the screen it sounds more like an issue of the terminal application than the cli library. The only limitations it has are the serial input buffer size (was for example 768 bytes in mbed-os-cliapp at some point), mbed-client-cli input buffer size (2000 bytes by default), and how fast the mcu can move the bytes out of one to the other. For example on a K64F with those buffer sizes pasting in a string of up to around 1500 characters at once worked fine. (Though admittedly that may have been with echo off.. It's been a while.)

tommikas · 2018-08-23T06:47:28Z

source/ns_cmdline.c

    }
    return NULL;
 }
+static const char* strrevchr(const char* from, const char* to, const char c)


What exactly is this function meant to do? Find the last occurrence of a character in a string?

exactly - between two pointers

tommikas · 2018-08-23T09:34:55Z

source/ns_cmdline.c

+  }
+  while((from > to) && (*from == c)) {
+    from--;
+  }


Doesn't the above loop skip past the first block of the searched for characters if it's the last character before NUL in the range?

For example if the string between the pointers is "aaaabaabbb\0\0" and you search for 'b'.

ah that's true - logic is really that it tries (fixing one bug soon) to find last character X but skip last ones if there is multiple--> is used to find "last" space....

strrevchr("aaaabaabbb\0\0" - 'b') --> bbb\0\0

add more documentation for function

tommikas · 2018-08-23T11:28:28Z

source/ns_cmdline.c

+  }
+  while (from > to) {
+    if (*from == ' ') {
+      return from+1;


So actually this returns a pointer to the character following the last space (ignoring trailing spaces).

Alright, makes sense now.

jupe · 2018-08-23T14:40:55Z

merged - please open ticket if you find any issues - as usual :).

jupe and others added 19 commits July 14, 2018 16:33

add linux example

18dc743

use ncurses which enables more realistic behaviour

6d697fe

tuning a bit

8f52699

tuning cli

f121500

* add option to ignore man pages by flag CMDLINE_INCLUDE_MAN=<bool> * add true and false commands and tests for available operators and, or, semicolon * add retcode tests * cleaning comments

add handlers for alt+<left>/<right> and ctrl+w

f89955b

small tuning

98727f5

* rename define CMDLINE_INCLUDE_MAN to MBED_CMDLINE_INCLUDE_MAN * remove underscore macro - use it always * add warning trace if max arguments reached * clean up some compiler warnings

refactoring escape logic

a94237f

use PS1 variable for prompt

4bbc563

update tests for unset and set cmd changes

fcc6fce

several improvements

5538db4

* allow to store also int values for variables * fixes alt+left behaviour * add "?" variable for last retcode * add function for request screen size

Merge branch 'master' into refactoring

03ac8ea

WIP: fixes for tests

5d57248

fix typo

77b0687

add tests for new features

061c4da

WIP: allows line feeds in variable

0e696ad

WIP: add more documentation and replace also integer variables

dac3a79

Merge branch 'master' into refactoring

a7f9ee4

Create .mbedignore

ad14d63

WIP

1ee7686

* add escaping support * prompt and retcode format is now as variable

Merge branch 'master' into refactoring

d27780a

jupe requested review from YifeiZuo01 and kuggenhoffen July 20, 2018 08:08

ignore builds

135a301

jupe commented Jul 20, 2018

View reviewed changes

test/Test.cpp

}

// // alias test

// extern void replace_alias(const char *str, const char *old_str, const char *new_str);

// TEST(cli, cmd_alias_1)

This comment was marked as resolved.

Sign in to view

jupe commented Jul 20, 2018

View reviewed changes

source/ns_cmdline.c

#define TRACE_GROUP "cmdL"

#ifndef MBED_CMDLINE_BOOT_MESSAGE

#define MBED_CMDLINE_BOOT_MESSAGE "ARM Ltd\r\n"

This comment was marked as resolved.

Sign in to view

jupe commented Jul 20, 2018

View reviewed changes

source/ns_cmdline.c

#endif

//include manuals or not (save memory a little when not include)

#define INCLUDE_MAN

#ifndef MBED_CMDLINE_INCLUDE_MAN

This comment was marked as resolved.

Sign in to view

YifeiZuo01 reviewed Jul 20, 2018

View reviewed changes

jupe force-pushed the refactoring branch from 7ee2377 to b31f72f Compare August 7, 2018 06:47

run valgrind memory leak detection with linux example

79d4ae9

jupe force-pushed the refactoring branch from b31f72f to 79d4ae9 Compare August 7, 2018 06:50

Jussi Vatjus-Anttila added 5 commits August 7, 2018 09:57

tune memtest and suppress compiler warnings

5c4da7a

tune memtest

e60eb2c

move leak-check to own stage and status

ad5af4d

cleanup some cppcheck warnings

b55fb67

replace trace with init screen in examples

6c5b19f

YifeiZuo01 previously approved these changes Aug 7, 2018

View reviewed changes

jupe requested a review from tommikas August 7, 2018 10:40

jupe added this to the v0.4.0 milestone Aug 7, 2018

jupe requested review from SeppoTakalo and removed request for SeppoTakalo, kuggenhoffen and tommikas August 7, 2018 11:02

Merge branch 'master' into refactoring

6684b93

jupe requested a review from mikter August 22, 2018 18:56

tommikas reviewed Aug 23, 2018

View reviewed changes

fix strrevchr

71b5c1f

add more documentation for function

jupe dismissed YifeiZuo01’s stale review via 71b5c1f August 23, 2018 10:13

Jussi Vatjus-Anttila added 3 commits August 23, 2018 13:20

one more fix for strrevchr

3078b33

Merge branch 'master' into refactoring

884e874

renamed strrevchr with find_last_space

6db2fda

tommikas reviewed Aug 23, 2018

View reviewed changes

jupe merged commit 1554081 into master Aug 23, 2018

jupe deleted the refactoring branch August 23, 2018 14:38

Improvements for cli behaviour #76

Improvements for cli behaviour #76

Uh oh!

Conversation

jupe commented Jul 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Status

Migrations

Description

Todos

Uh oh!

jupe commented Jul 20, 2018

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

YifeiZuo01 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YifeiZuo01 left a comment

Choose a reason for hiding this comment

Uh oh!

mikter commented Aug 23, 2018

Uh oh!

tommikas commented Aug 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jupe commented Aug 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jupe commented Jul 17, 2018 •

edited

Loading