Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added help terminal command #182

Merged
merged 4 commits into from
Sep 25, 2024
Merged

Conversation

nhjschulz
Copy link
Collaborator

@nhjschulz nhjschulz commented Sep 17, 2024

Added new command help to print the supported commands list.

Side changes:

  • Store cmd list in a table instead of if-else chain testing.
  • Avoid unneeded dynamic construction of static variables (strlen calls)
  • Read up to 12 bytes in process ( 32bit aligned to avoid gap byte loss for no gain).

Added new command help to print the supported commands list.

Side changes:
* Store cmd list in a table instead of if-else chain testing.
* Avoid unneeded dynamic construction of static variables (strlen calls)
* Read up to 12 bytes in process ( 32bit aligned to avoid gab byte loss of no gain).
@nhjschulz nhjschulz linked an issue Sep 17, 2024 that may be closed by this pull request
Copy link
Owner

@BlueAndi BlueAndi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main problem with the PR will be that it doesn't fit anymore. IMHO the threshold is around 99.5%. Maybe we should think about having the mini terminal as service, because the service can be easily removed by configuration. The drawback might be of course that it won't be available for some board configurations.

src/Common/MiniTerminal.cpp Outdated Show resolved Hide resolved
src/Common/MiniTerminal.h Outdated Show resolved Hide resolved
@nhjschulz
Copy link
Collaborator Author

The main problem with the PR will be that it doesn't fit anymore. IMHO the threshold is around 99.5%. Maybe we should think about having the mini terminal as service, because the service can be easily removed by configuration. The drawback might be of course that it won't be available for some board configurations.

I think it adds less then 200 bytes, but I can optimize further to be almost size neutral. It should even shrink if we kick out "ping" which is kind of obsolete if you have "help" also. Let me know if I should do this.

@BlueAndi
Copy link
Owner

The main problem with the PR will be that it doesn't fit anymore. IMHO the threshold is around 99.5%. Maybe we should think about having the mini terminal as service, because the service can be easily removed by configuration. The drawback might be of course that it won't be available for some board configurations.

I think it adds less then 200 bytes, but I can optimize further to be almost size neutral. It should even shrink if we kick out "ping" which is kind of obsolete if you have "help" also. Let me know if I should do this.

Removing "ping" is a good idea, because of the "help" command. I think with that we come back to previous size. I guess I will remove things like the reset monitor in the future, because it just helps for debugging issues. Need to think about other features too.

* Remove ping command (now there is help instead)
* Don't store cmd string length, but calc it on enter key
* Remove unneeded if's

This version need ~80 bytes less flash space then before.
@nhjschulz
Copy link
Collaborator Author

nhjschulz commented Sep 24, 2024

The main problem with the PR will be that it doesn't fit anymore. IMHO the threshold is around 99.5%. Maybe we should think about having the mini terminal as service, because the service can be easily removed by configuration. The drawback might be of course that it won't be available for some board configurations.

I think it adds less then 200 bytes, but I can optimize further to be almost size neutral. It should even shrink if we kick out "ping" which is kind of obsolete if you have "help" also. Let me know if I should do this.

Removing "ping" is a good idea, because of the "help" command. I think with that we come back to previous size. I guess I will remove things like the reset monitor in the future, because it just helps for debugging issues. Need to think about other features too.

The latest version is ~80 bytes less ROM than the one before the PR (adding text + data)
text data bss dec hex filename
1131614 330920 1173405 2635939 2838a3 .\firmware.elf (version before)
1131434 331012 1173381 2635827 283833 .\firmware.elf (with this PR)

src/Common/MiniTerminal.cpp Outdated Show resolved Hide resolved
src/Common/MiniTerminal.cpp Show resolved Hide resolved
@BlueAndi BlueAndi merged commit cca4463 into BlueAndi:Development Sep 25, 2024
27 checks passed
@BlueAndi
Copy link
Owner

Thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Add "help" command to terminal
2 participants