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

Correct parameters name in ParameterLoader #2458

Merged

Conversation

JoaoMario109
Copy link
Contributor

Change to use correct parameters values and units instead of just showing the raw enumeration value

Closes #2447

const paramValueText = printParam(parameter)
const paramUnitsText = parameter?.units ? `[${parameter.units}]` : ''

return paramValueText + paramUnitsText
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return paramValueText + paramUnitsText
return paramValueText + ' ' + paramUnitsText

I don't remember the correct syntax, but the idea is to have something like "10 [A]" instead of "10[A]".

edit: it might be better to just prepend a space on the paramUnitsText, otherwise we would have a space appended to the strings that don't have units.

Copy link
Collaborator

@joaoantoniocardoso joaoantoniocardoso left a comment

Choose a reason for hiding this comment

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

I'm not against the proposal, but I'd prefer to have a separate column with the units instead of having it appended to each value.

@JoaoMario109
Copy link
Contributor Author

I'm not against the proposal, but I'd prefer to have a separate column with the units instead of having it appended to each value.

I used this way to keep concise with the already existing implementation in

{{ printParam(item.item) }} {{ item.item.units ? `[${item.item.units}]` : '' }}

But is actually easy to change if you guys think its better

@patrickelectric
Copy link
Member

@JoaoMario109 can you rebase over master ?

* Change to use correct parameters values and units instead of just
  showing the raw enum value
@patrickelectric patrickelectric merged commit 1a92152 into bluerobotics:master Mar 26, 2024
6 checks passed
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.

Load default parameters should show a enum names over values
3 participants