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.

Sign up for GitHub

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

Jump to bottom

Feature/temperature reading #430

Merged
merged 7 commits into from
Nov 28, 2023
Merged

Feature/temperature reading #430

merged 7 commits into from
Nov 28, 2023

Conversation

MSECode
Copy link
Contributor

@MSECode MSECode commented Oct 24, 2023 •
edited
Loading

This PR brings the following changes:

  • Add former message to MC polling CAN message section eocanprotMCpolling_former_POL_MC_CMD__SET_TEMPERATURE_LIMIT used for sending to the 2FOC the temperature limits coming from the robots-configuration files and converted to raw values
  • Update method Motor_config_2FOC() for passing to the 2FOC the temperature limits when doing the motor configuration and also define the method Motor_config_motor_max_temperature_2FOC for doing the same but outside the configuration of the whole motor
  • Send temperature overcoming error when registered the raising on the OverHeatingFailure motor fault state bit coming from the 2foc
  • Update the MC period message eocanprotMCperiodic_parser_PER_MC_MSG__ADDITIONAL_STATUS for keeping the motor temperature value in the ADDITIONAL_STATUS info updated form the 2foc
  • Add temperature low level changes to v3.3 of the 2foc

Here a small flow diagram showing how the low level code is working on the FOC and EMS side

image

@MSECode MSECode requested review from valegagge and ale-git October 24, 2023 10:22
@MSECode MSECode self-assigned this Oct 24, 2023
@MSECode MSECode marked this pull request as draft October 24, 2023 10:23
@marcoaccame marcoaccame self-requested a review October 26, 2023 09:11
@marcoaccame
Copy link
Contributor

Hi @MSECode, I will wait to review the PR until ready and mergeable.

However, I read:

  • Update the MC period message eocanprotMCperiodic_parser_PER_MC_MSG__ADDITIONAL_STATUS for keeping the motor temperature value in the ADDITIONAL_STATUS info updated form the 2foc

this message is emitted also by amcbldc and by amc2c, so pls take care also of that.

@MSECode
Copy link
Contributor Author

MSECode commented Oct 26, 2023 •
edited
Loading

Hi @MSECode, I will wait to review the PR until ready and mergeable.

However, I read:

  • Update the MC period message eocanprotMCperiodic_parser_PER_MC_MSG__ADDITIONAL_STATUS for keeping the motor temperature value in the ADDITIONAL_STATUS info updated form the 2foc

this message is emitted also by amcbldc and by amc2c, so pls take care also of that.

Sure, I will updated their fw and I'll check compilation when opening PR for icub-firmware-build.
And anyway I added this if clause: https://github.com/robotology/icub-firmware/pull/430/files#diff-61fe10ec2d13efb162a3c19608541ffe1857ad699176dc1fd7c9ffad915f4b3dR465
to be sure that I'm gonna parse the new short on data[2] and data[3] iff we are using a foc type board

@MSECode MSECode force-pushed the feature/temperatureReading branch from 1ea919d to 0bc2b91 Compare October 26, 2023 09:42
@valegagge
Copy link
Member

We need to keep this issue in draft until the temperature reading feature doesn't pass tests on robots.

@MSECode MSECode force-pushed the feature/temperatureReading branch from ab7961e to 4e77122 Compare October 30, 2023 13:36
@valegagge
Copy link
Member

Hi @MSECode, I will wait to review the PR until ready and mergeable.

However, I read:

  • Update the MC period message eocanprotMCperiodic_parser_PER_MC_MSG__ADDITIONAL_STATUS for keeping the motor temperature value in the ADDITIONAL_STATUS info updated form the 2foc

this message is emitted also by amcbldc and by amc2c, so pls take care also of that.

Hi @marcoaccame,
I verified in the model of amcbldc and, if I'm not mistaken, the amcbldc doesn't send the additional status.

image

@valegagge valegagge force-pushed the feature/temperatureReading branch from 4e77122 to d3bb715 Compare November 3, 2023 11:51
valegagge
valegagge requested changes Nov 3, 2023
View reviewed changes
Copy link
Member

@valegagge valegagge left a comment

Choose a reason for hiding this comment

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

request some minor changes. Anyway good work

emBODY/eBcode/arch-arm/embobj/plus/mc/Motor.c Show resolved Hide resolved
emBODY/eBcode/arch-arm/embobj/plus/mc/Motor.c Outdated Show resolved Hide resolved
emBODY/eBcode/arch-arm/libs/highlevel/abslayer/ipal/ipal-extra/ipal_ipiit_storage.c Outdated Show resolved Hide resolved
emBODY/eBcode/arch-dspic/board/2foc/appl/2FOC-V3/src/can_icubProto_trasmitter.c Show resolved Hide resolved
@valegagge
Copy link
Member

Hi @MSECode,
I checked the documentation. It is not correct:
image

  1. the X value is not present, as you can see from the code you developed in the 2foc CAN parser
    image
  2. the hardware limits in uint16_t so the blu rectangular should be smaller as in other messages with ARGS of the same size

Please update it accordingly. Thanks! :)

@MSECode MSECode force-pushed the feature/temperatureReading branch from d1ff1c8 to 37306d4 Compare November 15, 2023 07:52
@MSECode
Started work on temperature reading
b3fbe10
@MSECode
Update confiuration and diagnostic-error methods for overheating
a847278 
Update mc controller for temperature limits
Update firmware 2foc and ems4 with latest parsing and forming of CAN temp messages
Update temperature reading code
@MSECode
Update transmission of temperature CAN message on 2foc
acaed35
@MSECode
Update CAN protocol motorboards documentation
15086d2 
Update versioning for uploading new release
Udpate CAN protocol motorboards documentation and aling SET_TEMPERATURE with new eth_mc_messaging api
Add default value for Temperature before overriding from sensor reading
Set def value for temp to -500
Set correct temperature reading error to -18
@MSECode MSECode force-pushed the feature/temperatureReading branch from 37306d4 to 15086d2 Compare November 23, 2023 16:48
@MSECode MSECode mentioned this pull request Nov 27, 2023
Merged
@MSECode MSECode requested a review from valegagge November 27, 2023 07:52
valegagge
valegagge approved these changes Nov 27, 2023
View reviewed changes
Copy link
Member

@valegagge valegagge left a comment

Choose a reason for hiding this comment

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

LGTM! Good work! 🚀

@valegagge valegagge marked this pull request as ready for review November 27, 2023 14:02
marcoaccame
marcoaccame approved these changes Nov 27, 2023
View reviewed changes
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.

it is ok, thanks

@MSECode
Keep CAN protocol version to 1.6
e33144b 
Release foc version v3.3.22
@MSECode MSECode force-pushed the feature/temperatureReading branch from 4530179 to e33144b Compare November 28, 2023 11:50
@MSECode MSECode mentioned this pull request Nov 28, 2023
Merged
@marcoaccame marcoaccame merged commit f2700ea into robotology:devel Nov 28, 2023
@MSECode MSECode deleted the feature/temperatureReading branch December 13, 2024 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcoaccame marcoaccame marcoaccame approved these changes

@valegagge valegagge valegagge approved these changes

@ale-git ale-git Awaiting requested review from ale-git

Assignees

@MSECode MSECode

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MSECode @marcoaccame @valegagge