UPB-CS-OpenSourceUpstream / tock

A secure embedded operating system for microcontrollers
https://www.tockos.org
Other
2 stars 6 forks source link

Implement command history for process console #2

Closed alexandruradovici closed 1 year ago

alexandruradovici commented 1 year ago

The process_console (shell) provided by Tock does not have a command history. Many users are trying to use the up and down arrow keys. The implementation of a limited command history would be useful.

mihai-negru commented 1 year ago

I have two implementations in mind of the command history:

What will be the best aproach for this issue? Thanks for the answer

alexandruradovici commented 1 year ago

As you do not have a file system in Tock, the second approach is feasible. The String type is not available as dynamic allocation is not allowed within the Tock kernel, you can use [u8; N] (array of bytes).

mihai-negru commented 1 year ago

As you do not have a file system in Tock, the second approach is feasible. The String type is not available as dynamic allocation is not allowed within the Tock kernel, you can use [u8; N] (array of bytes).

Hi, I have some questions related to the issue:

Thank you for answers

alexandruradovici commented 1 year ago
* I have found that in bash up and down arrow are mapped as ^[[A and ^[[B, can I assume that it works alike on the tock system or not?

I think so.

  • Also am I allowed to modify the ProcessConsole trait?

I think that you mean the ProcessConsole struct, yes you can make modifications to it

  • How many commands should I record maximum, would be ten enough?

I suggest we let the user decide by adding a const generic to the structure.

  • What would be the best, to implement an array od bytes and to store all the commands and to add a special character like null terminator to signal that the command has ended, or a matrix of u8?

What about having a struct like

struct Command<const LEN: usize> {
buffer: [u8; LEN],
len: usize
}
* I have a board to test the implemetation, but is there any method to test my implementation without using a board?

There does not seem to be one, as far as I know the serial port reading does not work in an emulator. Maybe it does in qemu, simulating a risc-v processor.

mihai-negru commented 1 year ago

Hi, I have same questions related to thie issue.

I created a structure for the command and created also a history structure in order to keep track of the commands inserted in the list, I had for the command structure to implement the Copy and Clone trait in order to initialize an array of command and then a new command is inserted I just change the buffer of bytes. However I have a problem related on this implementation.

I have to initialize a new field in the ProcessConsole structure

ProcessConsole {
// some fields
history: TakeCell<'what-label-should-be-here, HistoryStruct>
// other fields
}

I thought that TakeCell would do the trick becuase in all the methods related to ProcessConsole the instance of the strucutre is sent as &self.

I initialize the history field as:

ProcessConsole{
history: TakeCell::new(HistoryStruct::new())
}

Which will create an array with Commands Structure and will take care of managing the commands.

My problem is I do not know how to create a static instance of the History structure, or am I really supposed to create it with a static lifetime, should I create it with the lifetime of the ProcessConsole?

Also if I give a static lifetime for the history structure should I initialize it in the same file as ProcessConsole' buffers are created?

Whoukd also be good to create another file where to move all the History structure implementation in order not to polute the process console code ?

Also If the History Structure does not need a static lifetime, how should I initialize it because TakeCell::new(&mut HistoryStructure::new()) does not work even if I do something like this

let mut ht: History = HistoryStructure::new();
ProcessConsole {
    history: TakeCell::new(&mut ht),
}
alexandruradovici commented 1 year ago

I thought that TakeCell would do the trick becuase in all the methods related to ProcessConsole the instance of the strucutre is sent as &self.

Due to https://github.com/tock/tock/commit/5f7246d4af139864f567cebf15bfc0b49e17b787, MapCell might be a better idea, but this is something that you can worry about later.

My problem is I do not know how to create a static instance of the History structure, or am I really supposed to create it with a static lifetime, should I create it with the lifetime of the ProcessConsole?

I think the buffer can have a generic lifetime ('a), as it is never used outside the lifetime of the ProcessConsole. You can use the lifetime <'a> of the process console. The problem that you might encounter is that TakeCell and MapCell take mutable references to their wrapped data, instead of taking ownership (like Cell does). This means that somebody needs to take ownership for the data type. My impression is that this is only going to work with a 'static lifetime. This is why Process Console is initialized with 'a as 'static.

Also if I give a static lifetime for the history structure should I initialize it in the same file as ProcessConsole' buffers are created?

As this requires using static_init!, it cannot be instantiated in the capsule, it has to come from an unsafe environment, like the main function in main.rs. You will have to modify the Process Console's component https://github.com/tock/tock/blob/master/boards/components/src/process_console.rs. This means adding the buffer as a parameter to the new function and using static_init! in finalize to instantiate it and send it to new.

Whoukd also be good to create another file where to move all the History structure implementation in order not to polute the process console code ?

No, I don't think so.

Also If the History Structure does not need a static lifetime, how should I initialize it because TakeCell::new(&mut HistoryStructure::new()) does not work even if I do something like this

This is the problem that I explained earlier.

I suggest opening a draft pull request so we can discuss the code.

mihai-negru commented 1 year ago

I opened a pull request here https://github.com/UPB-CS-OpenSourceUpstream/tock/pull/16

This pull for now has a compile error