-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
MCP9600 UORB implementation #15605
MCP9600 UORB implementation #15605
Conversation
[Experimental Bot, please feedback here] Fill In The Commit Message: This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary. The PR description is a good start but needs improvement to fully meet the NuttX requirements. Here's a breakdown of what's missing and how to improve it: Summary:
Impact:
Testing:
Example of Improved Testing Section: Testing I confirm that changes are verified on my local setup and work as intended:
Testing logs before change (using legacy driver):
Testing logs after change (using UORB):
Further testing included setting different thermocouple types (types 2 and 3) and verifying the temperature readings against a reference thermometer. Different sampling rates were also tested (1Hz, 10Hz, and 50Hz) to confirm correct operation. A test with the sensor disconnected was performed, resulting in expected error messages being logged. By addressing these points, you'll significantly improve the clarity and completeness of your PR, making it easier for reviewers to understand and approve your changes. Remember to also follow the NuttX coding style and commit message guidelines. |
@linguini1 let's rebase on #15610, using new type: SENSOR_TEMPERATURE |
4847cf5
to
a370c47
Compare
beb2e4f
to
b342566
Compare
…r suggestions on PR apache#15525.
b342566
to
95c4495
Compare
Summary
This PR is from InSpace.
This PR moves the MCP9600 sensor implementation over to the UORB framework instead of the legacy structure, as per suggestions on #15525.
This also adds a dedicated
SNIOC
command for configuring the thermocouple type of a device (for amplifiers which support multiple types).I have also updated the documentation of the MCP9600 to reflect the changes.
Finally, this also changes the
SENSOR_TYPE_AMBIENT_TEMPERATURE
macro toSENSOR_TYPE_TEMPERATURE
macro, since the distinction is not required and prevents confusion about some sensor topics being for ambient temperature while others measure the direct temperature of an object.I have deliberately split the PR into two commits since renaming the macro I believe deserves its own distinct commit with a different message.
Impact
This improves the MCP9600 because it is now compatible with the more powerful UORB framework. This extension also paves the way for other thermocouple amplifiers and object temperature measuring sensors to be integrated into NuttX.
The changing of the macro name was discussed in the developer forums and suggested as an alternative to creating a new type which would be functionally the same as the temperature type, just by another macro name. I have modified all instances of the macro name in the kernel. This should not affect the user code since there are also no applications in the NuttX apps repository that mention the macro by name, but I may have overlooked some other impact.
Testing
Testing was performed similarly to the MCP9600 legacy driver but this time by reading measurements with the
uorb_listener
application. Rates were configurable and measurements were displayed properly.I have also built the documentation locally and verified that it renders correctly in the browser.