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 default values for FT58 and FT45 #780

Merged
merged 4 commits into from
Jan 21, 2022
Merged

added default values for FT58 and FT45 #780

merged 4 commits into from
Jan 21, 2022

Conversation

triccyx
Copy link
Member

@triccyx triccyx commented Jan 17, 2022

@marcoaccame
I have added two new params:

  • j4 set the gains to FT45 default
  • j5 set the gains to FT58 default

-j is still present and is mapped on -j4

Could you verify if this approach is good for you? Otherwise, we can discuss an alternative.

@davidetome @pattacini

@triccyx triccyx marked this pull request as ready for review January 18, 2022 09:49
@pattacini
Copy link
Member

Hi @triccyx

What's the rationale for

  • j4 set the gains to FT45 default
  • j5 set the gains to FT58 default

Why don't we go for j45 and j58 instead?

@triccyx
Copy link
Member Author

triccyx commented Jan 18, 2022

It is a mere technical problem.
I have reused the j parameters and created a couple of other 2 parameters 4 and 5. So used together become -j4 and -j5.
It is not possible to create a parameter with more than 1 letter/digit.

@triccyx
Copy link
Member Author

triccyx commented Jan 18, 2022

But it occurred to me now that I can also give a value to the j param like this
-j FT45
perhaps is better

@pattacini
Copy link
Member

perhaps is better

Definitely better, yeah 👍🏻

@triccyx
Copy link
Member Author

triccyx commented Jan 18, 2022

perhaps is better

Definitely better, yeah 👍🏻

Ok, I check how to change the code.

@triccyx
Copy link
Member Author

triccyx commented Jan 18, 2022

All the available letters for parameters are already used (a...z) also some numbers! In order to maintain retro compatibility for the old -j parameters I choose the following solution:
-3 FT45 set the gains to FT45 default
-3 FT58 set the gains to FT58 default
-j set the gains to FT45 default for retro compatibility
Unfortunately with the current library, used for parameters reading, seems that is not possible to specify a parameter (-j) with and without a value another way the best things would be:
-j FT45 set the gains to FT45 default
-j FT58 set the gains to FT58 default
-j set the gains to FT45 default

src/tools/FirmwareUpdater/main.cpp Outdated Show resolved Hide resolved
src/tools/FirmwareUpdater/main.cpp Outdated Show resolved Hide resolved
src/tools/FirmwareUpdater/main.cpp Outdated Show resolved Hide resolved
src/tools/FirmwareUpdater/main.cpp Outdated Show resolved Hide resolved
src/tools/FirmwareUpdater/main.cpp Outdated Show resolved Hide resolved
@pattacini pattacini merged commit 6dde053 into robotology:master Jan 21, 2022
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.

4 participants