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
add getCoreTemp #12608
add getCoreTemp #12608
Conversation
This comment has been minimized.
This comment has been minimized.
AUTOMERGE: (FAIL)
|
src/main/msp/msp.c
Outdated
// Added in API version 1.46 | ||
// Write CPU temp | ||
if (cmdMSP == MSP_STATUS_EX) { | ||
#ifdef USE_ADC_INTERNAL | ||
int16_t coretemp = getCoreTemperatureCelsius(); | ||
#else | ||
int16_t coretemp = 0; | ||
#endif | ||
sbufWriteU16(dst, coretemp); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we have MSP_STATUS and MSP_STATUS_EX almost the same? What is the difference between both? I see the PID profile count vs gyro cycle time. Something more? Why we don't send the coretemp for both?
Is not a problem of your code, only trying to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't say, believe its a matter of backward compability. But also wonder, if this are not an issue, since we have changed API to 1.46. In betaflight_configurator, I will check for API 1.46 if the value can be shown.
No problem from my point of view to send the coreTemp for both MSP_STATUS and MSP_STATUS_EX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed, now no difference MSP_STATUS and MSP_STATUS_EX
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@haslinghuis hope you agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 we should drop MSP_STATUS_EX and consolidate functionality using updated MSP_STATUS
@klutvott123 does this impact LUA? Thoughts?
MSP_STATUS_EX was introduced long time ago as MSP_STATUS is MultiWii compliant 😕
if (semver.gte(FC.CONFIG.apiVersion, API_VERSION_1_32)) {
MSP.send_message(MSPCodes.MSP_STATUS_EX, false, false);
} else {
MSP.send_message(MSPCodes.MSP_STATUS, false, false);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MultiWii Homepage look very obsolute ...
Now also repported from MSP_STATUS
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Míguel Ángel Mulero Martínez <mcgivergim@gmail.com>
Do you want to test this code? Here you have an automated build: |
need test, maybe something are wrong, I cant get the image STM32H743 from the test run to work |
* add getCoreTemp * add ifdef * Update msp.c Now also repported from MSP_STATUS * Update src/main/msp/msp.c Co-authored-by: Míguel Ángel Mulero Martínez <mcgivergim@gmail.com> --------- Co-authored-by: Míguel Ángel Mulero Martínez <mcgivergim@gmail.com>
Add getCoreTemperature to MSP_STATUS_EX call, to show CPU temperature in configurator.
Last 16bit unsigned in MSP_STATUS_EX