process_console command history

[CosminGGeorgescu]

Pull Request Overview

This pull request adds an easy-to-use Bash-like command history for the process console.

Added features

• User can enable/disable the command history from the board's main.rs.
• If enabled, user can scroll up or down by pressing up and down arrow keys respectively.
• Pressing on accident up or down arrow saves the unfinished command and can be fetched back by going to the bottom of command history.
• Modifying a command from history is also saved (if accidentally pressed up or down arrow).
• Empty commands (whitespace-filled) don't get registered in the command history.
• Invalid commands also are saved into the history structure.

Code size for IMIX binary build with enabled/disabled command history:

Build	.text	.data	.bss	.dec
Tock master	174468	0	29592	204060
Enabled with default size	175492	0	28944	204436
Disabled	174292	0	28576	202868

Testing Strategy

The implementation of the command history was tested on the nrf52840dk and esp32-c3-devkitM-1 boards.

Files changed

capsules/process_console.rs ~ implementation for command history

boards/components/process_console.rs ~ added a new branch to process_console_component_static macro for easily enable/disable the command history

Documentation Updated

Updated the documentation in doc/Process_Console.md accordingly :

• Code support on how to enable/disable the command history
• How to use command history
• Support for selecting a custom size for the command history

Formatting

• [x] Ran make prepush

[valexandru]

Also, a recommendation is to run make prepush before pushing the code.

[alexandruradovici]

You can both chat using https://gitter.im/UPB-CS-OpenSourceUpstream/community?utm_source=share-link&utm_medium=link&utm_campaign=share-link

[alexandruradovici]

Our suggestion is to do one of the following:

• as this pull request is more complete, @matrix22 clones this pull request branch from @CosminGGeorgescu, makes the changes to the data types and applies the const generics (<const ...>) to it, and resubmits the pull reqeuest
• @CosminGGeorgescu gives access to @matrix22 to his repository so that he can push the changes directly to this pull request

[CosminGGeorgescu]

I would go with the second bullet-point, if it's ok with you @Matrix22.

[mihai-negru]

I would go with the second bullet-point, if it's ok with you @Matrix22.

Yes, it is ok, i will try to make the modifications on Sunday if it's alright?

[mihai-negru]

I would go with the second bullet-point, if it's ok with you @Matrix22.

@CosminGGeorgescu I created this char room to discuss futher implementations

[mihai-negru]

Testing the implementation on the nrf52840dk chip I found that typing the same command multiple times when pressing the up arrow promts all the inserted commands multiple times even though they are the same, I think it would be better if the command equals to the previous command we should not to insert into the structure. @CosminGGeorgescu what do you thing about this?

[CosminGGeorgescu]

Intended as to mimic an unconfigured Bash. However, if @alexandruradovici or @valexandru object, a ignoredups / erasedups behaviour can be implemented.

[alexandruradovici]

@CosminGGeorgescu do you have any branch protection? @Matrix22 gers an error when trying to push to.

[CosminGGeorgescu]

@CosminGGeorgescu do you have any branch protection? @Matrix22 gers an error when trying to push to.

Should be good by now.

[alexandruradovici]

Please resolve the conflicts so we can run testing.

[alexandruradovici]

Please rebase or resolve conflicts this to be able to run the actions. The new process_console.rs has different documentation.

[mihai-negru]

Please rebase or resolve conflicts this to be able to run the actions. The new process_console.rs has different documentation.

Should I add the new implemented features in the process_console.rs or to write them in doc/processConsole because I see that all documentation was moved in the doc folder

[alexandruradovici]

Should I add the new implemented features in the process_console.rs or to write them in doc/processConsole because I see that all documentation was moved in the doc folder

ProcessConsole.md is the place to add the documentation. Please add the arrows and the idea that an empty command list will generate less code size.

[alexandruradovici]

To rebase this, use the following:

• Make a copy of your branch
• Update your master using github (Sync Fork).
• Run

git checkout master
git pull
git checkout process_console_history
git rebase master
git push --force

[mihai-negru]

To rebase this, use the following:

• Make a copy of your branch
• Update your master using github (Sync Fork).
• Run

git checkout master
git pull
git checkout process_console_history
git rebase master
git push --force

The git pull on the master branch tells me that Already up to date, however the Process_Console.md does not appear, should I create it and copy the content from the Main stream?

[alexandruradovici]

The git pull on the master branch tells me that Already up to date, however the Process_Console.md does not appear, should I create it and copy the content from the Main stream?

Did you sync your master (@CosminGGeorgescu's master) with the upstream master?

[mihai-negru]

The git pull on the master branch tells me that Already up to date, however the Process_Console.md does not appear, should I create it and copy the content from the Main stream?

Did you sync your master (@CosminGGeorgescu's master) with the upstream master?

I worked directly on the @CosminGGeorgescu 's master without forking

[alexandruradovici]

I worked directly on the @CosminGGeorgescu 's master without forking

Using github's web interface, go to Sync fork and use Update master.

[mihai-negru]

I worked directly on the @CosminGGeorgescu 's master without forking

Using github's web interface, go to Sync fork and use Update master.

So I updated master and solved rebase conflics however when I run now `make prepush it gives me 1146 errors because of the kernel package:

warning: unused import: `core::convert::From`
  --> capsules/src/text_screen.rs:15:5
   |
15 | use core::convert::From;
   |     ^^^^^^^^^^^^^^^^^^^

error[E0220]: associated type `Ticks` not found for `A`
  --> capsules/src/test/random_alarm.rs:15:23
   |
15 |     expected: Cell<A::Ticks>,
   |                       ^^^^^ associated type `Ticks` not found

Some errors have detailed explanations: E0220, E0412, E0432, E0433.
For more information about an error, try `rustc --explain E0220`.
warning: `capsules` (lib) generated 7 warnings
error: could not compile `capsules` due to 1146 previous errors; 7 warnings emitted
make[1]: *** [Makefile:145: allcheck] Error 101
make[1]: Leaving directory '/home/tock/tock'
make: *** [Makefile:374: ci-job-syntax] Error 2

This are just some errors

However my board flashes successfully

[alexandruradovici]

@CosminGGeorgescu's master branch seems to be in sync with upstream tock. Right now did you follow these steps:

git checkout master
git pull # this should update your master
git checkout process_console_history
git rebase master

Did you clone @CosminGGeorgescu's repository?

[mihai-negru]

@CosminGGeorgescu's master branch seems to be in sync with upstream tock. Right now did you follow these steps:

git checkout master
git pull # this should update your master
git checkout process_console_history
git rebase master

Did you clone @CosminGGeorgescu's repository?

All solved I had to update my system and the rust compiler

[alexandruradovici]

Take a look at the markdown TOC, you have the command that you can run to generate the TOC.

[alexandruradovici]

This seems to work nice. Please address @valexandru request, to store as the first command the command that is currently being written, so that when the user presses up and the down again he doesn't loose the command that he has just written.

[alexandruradovici]

This looks really good. What we need now is to prove that it does not affect code size when len is 0. Please do the following:

• build this for imix
• build this with len 0 for imix
• build the master branch for imix

Write the code sizes that you get for all of these. Try the same for the microbit, the problem here that code gets somehow rounded up and seems to be the same value.

[mihai-negru]

This looks really good. What we need now is to prove that it does not affect code size when len is 0. Please do the following:

• build this for imix
• build this with len 0 for imix
• build the master branch for imix

Write the code sizes that you get for all of these. Try the same for the microbit, the problem here that code gets somehow rounded up and seems to be the same value.

I am going to build the binary files, after implementing one last functionality for the history command (to register the command, when scrolling in the history and pressing backspace, becayse also should be considered as a modification), what do you think about this?

[alexandruradovici]

I am going to build the binary files, after implementing one last functionality for the history command (to register the command, when scrolling in the history and pressing backspace, becayse also should be considered as a modification), what do you think about this?

I;m not sure I fully understand what you want to do.

[mihai-negru]

I am going to build the binary files, after implementing one last functionality for the history command (to register the command, when scrolling in the history and pressing backspace, becayse also should be considered as a modification), what do you think about this?

I;m not sure I fully understand what you want to do.

For example user scrolls throught history ["Foo", ''Bar", "FooBar"] And stops at the "Bar" command Now with current implementation if user modifies the "Bar"command as:

• inserts new characters
• deletes some characters and after that add some new bytes

The command will be registred as unfinished and will be saved as 0 position of the command history

Now I want to do the same (to register the command) when the user just deletes some characters from the command without futher adding some new characters

[mihai-negru]

I am going to build the binary files, after implementing one last functionality for the history command (to register the command, when scrolling in the history and pressing backspace, becayse also should be considered as a modification), what do you think about this?

I;m not sure I fully understand what you want to do.

For example user scrolls throught history ["Foo", ''Bar", "FooBar"] And stops at the "Bar" command Now with current implementation if user modifies the "Bar"command as:

• inserts new characters
• deletes some characters and after that add some new bytes

The command will be registred as unfinished and will be saved as 0 position of the command history

Now I want to do the same (to register the command) when the user just deletes some characters from the command without futher adding some new characters

However this functionality could be achieved if user presses space after deleting some characters, the unfinished command will be saved without the last space character For example: "FooBar" -> [deletes so characters] -> "FooB" -> [presses spce] -> "FooB " -> [continues scrolling] => [unfinished command] "FooB"

[alexandruradovici]

Got it.

[alexandruradovici]

I assume this is not ready yet.

[mihai-negru]

I assume this is not ready yet.

Not yet, I am now working on it, I will push the desired functionality in the next comit

[mihai-negru]

I assume this is not ready yet.

Not yet, I am now working on it, I will push the desired functionality in the next comit

Now the functionalities are complete, I have teste them on my boards and seem to work nicely. Now I will build binaries in order to see if the executable size reducees upon custom history size of 0.

[mihai-negru]

This looks really good. What we need now is to prove that it does not affect code size when len is 0. Please do the following:

• build this for imix
• build this with len 0 for imix
• build the master branch for imix

Write the code sizes that you get for all of these. Try the same for the microbit, the problem here that code gets somehow rounded up and seems to be the same value.

So I have compiled the binaries for imix, microbit_v2 and nrf52840dk, these are the results:

running `stat -c %s tock/target/thumbv7em-none-eabi/release/<board_binary>`

IMIX build:
DEFAULT SIZE:   6213060
0 SIZE:     6156876

microbit-v2 build:
DEFAULT SIZE:   4284708
0 SIZE:     4229512

nrf52840dk build:
DEFAULT SIZE:   5708788
0 SIZE:     5652616

Seems even for microbit the the size gets reduced.

[alexandruradovici]

Please take the size that the make command prints. Don't forget to write the master branch size.

[mihai-negru]

These are the results by running make and extracting the output from it:

running `make`      text        data        bss     dec

IMIX build:
DEFAULT SIZE:              175492      0         28944      204436  
0 SIZE:                174292      0         28576      202868      

microbit-v2 build:
DEFAULT SIZE:              106500      0         16420      122920       
0 SIZE:                106500      0          16060      122560       

nrf52840dk build:
DEFAULT SIZE:               159748    4      26604      186356   
0 SIZE:                 159748      4          26244      185996       

As I see the text segment does not reduce upon size of 0 (just for imix)

Also, sorry for question, what do you mean by master branch size?

[alexandruradovici]

These are the results by running make and extracting the output from it:

running `make`        text        data        bss     dec

IMIX build:
DEFAULT SIZE:            175492      0         28944      204436  
0 SIZE:                  174292      0         28576      202868      

microbit-v2 build:
DEFAULT SIZE:            106500      0         16420      122920       
0 SIZE:                  106500      0          16060      122560       

Most probably the code size is rounded up.

nrf52840dk build:
DEFAULT SIZE:             159748    4      26604      186356   
0 SIZE:                   159748      4          26244      185996       

Most probably the code size is rounded up. Also, sorry for question, what do you mean by master branch size?

Use the master branch that Tock has now and that does not have the command history implemented.

[mihai-negru]

These are the results by running make and extracting the output from it:

running `make`        text        data        bss     dec

IMIX build:
DEFAULT SIZE:            175492      0         28944      204436  
0 SIZE:                  174292      0         28576      202868      

microbit-v2 build:
DEFAULT SIZE:            106500      0         16420      122920       
0 SIZE:                  106500      0          16060      122560       

nrf52840dk build:
DEFAULT SIZE:             159748    4      26604      186356   
0 SIZE:                   159748      4          26244      185996       

As I see the text segment does not reduce upon size of 0 (just for imix)

From Tock repository buildings:

text|data|bss|dec
IMIX:               174468       0   29592  204060
MICROBIT:      106500       0   17068  123568
NRF52840DK: 159748       4   27252  187004

As I see, the microbit and nrf52840 do not change their text sizes

However I look up on the main.rs for microbit and nrf52840dk and they use

// nordic
use nrf52840::gpio::Pin;
use nrf52840::interrupt_service::Nrf52840DefaultPeripherals;
use nrf52_components::{self, UartChannel, UartPins};

// microbit
use nrf52833::gpio::Pin;
use nrf52833::interrupt_service::Nrf52833DefaultPeripherals;

Where imix do not use them, could the code rounding to come from these packages?

[alexandruradovici]

This is fine, please write the pull request message and make a table with the code sizes for imix.

[mihai-negru]

This is fine, please write the pull request message and make a table with the code sizes for imix.

I cannot edit the pull request message, because @CosminGGeorgescu is the one who opened the pull request

[alexandruradovici]

Write a comment with the new message.

[mihai-negru]

Write a comment with the new message.

Pull Request Overview

The idea of this pull request is to set the foundation for building a command history for the ProcessConsole.

Functionalities

• User can enable/disable the command history from the main.rs if the board.
• If enabled, user can scroll up or down by pressing up and down arrow keys.
• Pressing accidentally on up or down arrow saves the unfinished command and can be fetched back by going to the bottom of command history.
• Modyfing a command from the history is also saved (if accidentally pressed up or down arrow).
• Empty commands (full of whitespaces or just empty) do not get registred in the command history.
• Unvalid commands also are saved into the history structure.

Code size for IMIX binary build with enabled/disabled command history:

Builds	text	data	bss	dec
Tock Master	174468	0	29592	204060
Enabled with default size	175492	0	28944	204436
Disabled	174292	0	28576	202868

Testing Strategy

The implementation of the command history was tested on nrf52840dk board.

Files changed

• capsules/process_console.rs
  • implementation for command history
• boards/components/process_console.rs
  • added a new branch to process_console_component_static marco for easy enabling/disabling command history

TODO or Help Wanted

N/A

Documentation Updated

Added new documentation support in doc/Process_Console.md:

• Code support on how to enable/disable the command history
• How to use command history
• Support for selecting a custom size for the command history

[CosminGGeorgescu]

Good to note that copying a formatted piece of text doesn't translate 1:1 upon pasting. Just updated the PR message. Mersi mult @Matrix22

[CosminGGeorgescu]

PR sent https://github.com/tock/tock/pull/3381.

[alexandruradovici]

Please write the full pull request message from here to upstream, including the code sizes and the fact that this changes does actually decrease code size if disabled.