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

2foc 3.36, mc4plus 3.95, mc2plus 3.74, amc 2.8, 2foc-special 3.136 #156

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

valegagge
Copy link
Member

This PR is related to robotology/icub-firmware#503 and robotology/icub-main#972.

The 2foc-special is useful for AMI demos when they don't need the overheating fault check.

cc @MSECode

@S-Dafarra
Copy link

Hi @valegagge, my two cents would be that it is better not to have duplicated 2foc FW. It can be difficult to maintain both, and I am afraid that in a not-so-far future we will simply forget what special means here.

During the last demos, we simply disabled the temperature sensors since it is:

  • faster
  • easier to switch back
  • more evident that there has been a modification.

The only time in which we used the special FW, we burnt down a motor as soon as we got home, and during the demo it turned out it was not useful to check the temperature.

In short, I would suggest to simply drop that FW.

@valegagge
Copy link
Member Author

Hi @valegagge, my two cents would be that it is better not to have duplicated 2foc FW. It can be difficult to maintain both, and I am afraid that in a not-so-far future we will simply forget what special means here.

During the last demos, we simply disabled the temperature sensors since it is:

  • faster
  • easier to switch back
  • more evident that there has been a modification.

The only time in which we used the special FW, we burnt down a motor as soon as we got home, and during the demo it turned out it was not useful to check the temperature.

In short, I would suggest to simply drop that FW.

Hi @S-Dafarra ,
I prepared the 2foc-special fw because we agreed to provide you a fw for the demo. Do you remember that in the last PI we also prepared a script to install that fw in user-friendly way?

Of course if you no longer use it I'm very happy to remove from the repo for all the reasons you mentioned! 😄

After the merge of this PR, I'm going to purge the repo from all references to this FW. (From script also)

@marcoaccame marcoaccame self-requested a review July 24, 2024 12:10
Copy link
Contributor

@marcoaccame marcoaccame left a comment

Choose a reason for hiding this comment

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

Hi @valegagge, pls remember to change the firmware.info.xml file and then we can merge.

@valegagge
Copy link
Member Author

Hi @valegagge, pls remember to change the firmware.info.xml file and then we can merge.

done here: valegagge@bd997c2

thanks!!!

cc @marcoaccame

Copy link
Contributor

@marcoaccame marcoaccame left a comment

Choose a reason for hiding this comment

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

ok, thanks

@marcoaccame marcoaccame merged commit 4afa2f5 into robotology:devel Jul 24, 2024
@valegagge valegagge deleted the 2focandQEncDiagnImprovements branch July 24, 2024 16:20
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