CLI and feedback enhancements by stevebroshar · Pull Request #461 · nxp-imx/mfgtools

stevebroshar · 2025-03-11T21:52:34Z

This is a big change so you may be hesitant to merge it. None the less and FWIW the tool is very useful, but I found the UX lacking including CLI, help info and user feedback. I reworked these aspects and as it was a significant change and the CLI code lacked structure (lacked componentization, uuu.cpp way too big, ...), I organized the code somewhat.

I started from NXP repo commit da3cd53 (16-May-2024, Fix PID 0159 device name from imx93 to imx91). Via backmerge, all files auto-merged except for uuu.cpp which had too many changes. To support review of the new CLI without deleting the old one, I renamed my uuu.cpp to cli.h so that uuu.cpp is mostly the same as it was. Can choose which CLI to use in main(). It's a half-measure but I didn't want to just delete the existing CLI code. I did update the old CLI to work with the other changes throughout.

There were about 20 commits after 16-may-2024 up to today (11-mar-2024). Since all files automerged other than uuu.cpp, only commits that modified uuu.cpp require manual merging. These are:

f764c01 fixes #443: -udev stdout should replace 70-uuu.rules rather than be appended to it
Resolution: Changes made manually in cli.h
5d77a61 Improve uuu help context
Resolution: No chagne required since new CLI code includes new help info
6c2141e Add dynamic MAX_PROGRESS_WIDTH global variable to handle long lines (#435)
Resolution: **Skipped since couldn't understand intent of change. FYI This change would go into HorizontalScrollingFormatter.h; not cli.h
09fe178 remove ref global varible g_verbose in bmap.cpp
Resolution: Changes made manually in cli.h
c031b2a move global g_bmap_mode into libuuu
Resolution: Changes made manually in cli.h

CHANGES
o Theme: Command based CLI
- New CLI that is command-based; instead of switches that control high-level modes
- Segregation between install operations and other operations. Most options only apply to install. Makes it clearer what options go with what operations.
- install [OPTIONS] auto-detect-file
- install [OPTIONS] -s script-file ARGS
- install [OPTIONS] PRO: ARGS
- ls-devices: replaces -lsusb
- ls-builtin [BUILTIN]: moved built-in list out of -h
- cat-builtin BUILTIN: replaces -bshow
- help CONTEXT: organizes help info more consistently
- Renamed -d to --continuous: 'deamon' is too gargony
- Renamed -s to -i|--interactive: 'shell' is too gargony
- Renamed -m to --filter-path: more readable
- Renamed -ms to --filter-serial: more readable
- All multi-char options require two dashes: more consistent with other tools
o Theme: Enhance overwrite-based feedback
- Fixed horizontal scrolling of active command; it was only scrolling one or two chars left
- Show final status as "Successful" instead of "Done" since is more useful to know succeeded; not just done
o Theme: Enhance append-based feedback
- Show waiting for device message after validation so that it's not showing when never actually waiting for a device
- NOTIFY_CMD_INFO messages came out without context and % complete partially overwrote it; looked bad
- Added color and spacing to enhance readability
o Theme: Enhance CLI and script processing UX
- Overhauled error handling and reporting for CLI and script processing
- Provide limited feedback for bad input instead of outputting pages of info which is off-putting and confusing. Show limited help unless user asks for details.
- Edited help content for clarity and organized for uniformity
o Theme: Simplify logic and enhance code quality
- For overwriting feedback, leave the cursor at the end of the status area instead of trying to move it below the area on exit
- Factored out much of uuu.cpp into new files since it was too large and consisting of rather unrelated logic
- Renamed types that mentioned built-in scripts to be more generic since they are used for custom scripts as well as built-in script
- Add logger class and singleton to centralize logging logic
- Removed "using std" to make code easier to move between .cpp and .h files
- Changed feedback output to stderr so that commands that output info that one might want to capture does not include the info (unless they also capture stderr). In particular, 'help udev' (old -udev) outputs info that one wants in a file, but they don't want the app title or the instructions which both now go to stderr.
- Added header comments for functions that I learned what they did
o Theme: Support doxygen
- Enable doxygen output by adding @file to source files
- Enable doxygen output by adding Doxyfile; didn't understand how the existing Doxyfile.in file works
o Theme: Tried to fix auto-complete
- Auto-complete disabled for Linux since fails for input like: uuu f.uuu foo
- Auto-complete disabled for Windows since crashes for input line: uuu -autocomplete
o Theme: devops enhancements
- Minor changes to support building outside CI build system

nxpfrankli · 2025-03-11T22:21:38Z

Thank you for your patches.

need add Signed-off-by: tag for each commit.
Can you split to some small pull request, we can go through one by one.
Avoid code style change and logic change together.
Remove debug code.

nxpfrankli · 2025-03-11T22:22:16Z

 *.snap
 node_modules
-build
+build


Create sperate patch for .gitignore file

nxpfrankli · 2025-03-11T22:25:50Z

 static bool g_small_memory = true;

+// [what is the point/value of this value?]
 #define MAGIC_PATH '>'


Mark avoid dead loop when search parent path

nxpfrankli · 2025-03-11T22:26:05Z


+/**
+ * @brie Base class for FS things [whatever 'FS' means/is].
+ */


remove [...]

nxpfrankli · 2025-03-11T22:26:52Z

+	 * Passing a bool like dir is bad style since it only controls flow.
+	 */
+	virtual int split(const string &path, string *dir_part, string *name_part, bool dir=false)
+	{


use seperate patch to change outbackfile-> dir_part, outfilename -> filename_part

nxpfrankli · 2025-03-11T22:27:14Z


 protected:
+	FSBasic() {} // enforce that this class is abstract base; only creatable via a superclass
 	const char * m_ext = nullptr;


This one should be sperate patch

nxpfrankli · 2025-03-11T22:28:04Z

-		set_last_err_string("FB: bmap file not found");
+		set_last_err_string("FB: bmap file not found: " + m_bmap_filename);
 		return -1;
 	}


avoid two line should sperate patch, show improve error message

nxpfrankli · 2025-03-11T22:28:20Z

 		nt.type = nt.NOTIFY_WAIT_FOR;
-		nt.str = (char*)"Wait for Known USB";
+		nt.str = (char*)"DWait for Known USB";
 		call_notify(nt);


Why need 'D'

nxpfrankli · 2025-03-11T22:28:59Z

+		"\tRegister-ArgumentCompleter -CommandName uuu -ScriptBlock {param($commandName,$parameterName,$wordToComplete,$commandAst,$fakeBoundParameter); " << path << " -autocomplete $parameterName }" << endl;
 #endif
-
 }


this part should be sperate patch

nxpfrankli · 2025-03-11T22:29:28Z

 	}
-	fprintf(file, ">");
+	//fprintf(file, ">");
 }


why comment this

nxpfrankli

Demostrate how to split/organize patches. tips: You use use "git add -p" to split patch. Need signed-of-by tag for each commits.

SteveBroshar added 30 commits February 19, 2025 16:08

CLI and feedback enhancements

ecae36e

refactoring

8be2c0c

Enhance script classes

76b3658

simplify script

5dfe40d

fix bugs

2278318

organize

084d9c6

support doxygen

ca0bd59

code format

95c50a8

define func instead of macro

e0b6c2a

fix linux build issues

a177e4f

use windows setenv func

0d1cb02

implement setenv for linux

7e74061

Updates for gcc

1274c94

update linux code for general changes

6a952e3

fix compiler warnings

a3497a3

feedback

5263452

error if no operation

ac94a69

protocol command help

f5c13b3

disable autocomplete for windows

5ca8c1d

disable auto-complete for linux

fed38ca

comments

f141786

logging

8f25328

simplify CLI handling by writing app title to stderr

59b35cc

simplify cli parsing

ddc5711

feedback

a09392a

remove 'v'

14fc152

make build work even without appveryer or git tags

3814eb7

Comments/feedback

72a2d64

Remove 'using std'

86333a1

new cli

8f3c267

SteveBroshar added 5 commits March 11, 2025 15:47

remove unused function

36829cb

remove first CLI rework attempt

27aca3b

help text

3fab0be

remove unused declaration

d2ee494

help info

647c156

nxpfrankli reviewed Mar 11, 2025

View reviewed changes

Comment thread .gitignore

*.snap

node_modules

build

build

Copy link
Copy Markdown

Contributor

nxpfrankli Mar 11, 2025

Choose a reason for hiding this comment

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

Create sperate patch for .gitignore file

nxpfrankli reviewed Mar 11, 2025

View reviewed changes

Comment thread uuu/buildincmd.cpp Outdated

}

fprintf(file, ">");

//fprintf(file, ">");

}

Copy link
Copy Markdown

Contributor

nxpfrankli Mar 11, 2025

Choose a reason for hiding this comment

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

why comment this

nxpfrankli reviewed Mar 11, 2025

View reviewed changes

SteveBroshar added 6 commits March 17, 2025 15:30

help info

615fb62

Show message instead of empty table for no devices found

2a47358

cleanup interactive mode feedback and code

3047e6a

help/feedback info

7323702

warn for empty input file

0f5c691

rename/shorten command to list devices

7af2439

nxpfrankli force-pushed the master branch 7 times, most recently from e55c395 to 42f13af Compare August 5, 2025 15:53

nxpfrankli force-pushed the master branch from 6747f1f to 230f1b1 Compare January 20, 2026 16:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI and feedback enhancements#461

CLI and feedback enhancements#461
stevebroshar wants to merge 91 commits intonxp-imx:masterfrom
stevebroshar:cli-and-feedback-enhancements

stevebroshar commented Mar 11, 2025

Uh oh!

nxpfrankli commented Mar 11, 2025

Uh oh!

nxpfrankli Mar 11, 2025

Uh oh!

nxpfrankli Mar 11, 2025

Uh oh!

nxpfrankli Mar 11, 2025

Uh oh!

nxpfrankli Mar 11, 2025

Uh oh!

nxpfrankli Mar 11, 2025

Uh oh!

nxpfrankli Mar 11, 2025

Uh oh!

nxpfrankli Mar 11, 2025

Uh oh!

nxpfrankli Mar 11, 2025

Uh oh!

nxpfrankli Mar 11, 2025

Uh oh!

nxpfrankli left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

stevebroshar commented Mar 11, 2025

Uh oh!

nxpfrankli commented Mar 11, 2025

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!

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!

nxpfrankli left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants