Skip to content

Use a global Config struct for arguments & accept log file as an argument - #17

Open
AJ-Williams1 wants to merge 2 commits into
masterfrom
aj/accept-logpath-argument
Open

AJ-Williams1 wants to merge 2 commits into
masterfrom
aj/accept-logpath-argument

Conversation

@AJ-Williams1

Copy link
Copy Markdown
Contributor

Problem

Firmware currently accepts command-line arguments for controlling policy-scale, kp-scale, and kd-scale. These arguments are parsed in robot_description.rs, rather than early on in main. This makes it difficult to add new arguments which affect other parts of the code--in this case an argument to specify where log files should be saved.

Solution

Arguments are now parsed first thing, in main, and placed into a Config struct. This struct is then passed through a series of functions to RobotDescription. I've also added an argument for the log file.

@AJ-Williams1
AJ-Williams1 requested a review from gitfish7 July 25, 2025 09:16

@gitfish7 gitfish7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, do we have build + run validation to ensure no regressions?

@gitfish7 gitfish7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes look good, let's fix the commit messages and post some validation! Will approve after

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

3 participants