Skip to content

drivers: add driver for MAX31865 resistance-to-digital converter#21498

Merged
gschorcht merged 4 commits intoRIOT-OS:masterfrom
dpproto:21362-add-driver-max31865
Jun 27, 2025
Merged

drivers: add driver for MAX31865 resistance-to-digital converter#21498
gschorcht merged 4 commits intoRIOT-OS:masterfrom
dpproto:21362-add-driver-max31865

Conversation

@dpproto
Copy link
Copy Markdown
Contributor

@dpproto dpproto commented May 17, 2025 •
edited
Loading

Contribution description

Implement a new driver for the MAX31865 resistance-to-digital converter, optimized for RTDs such as Pt100 or Pt1000 platinum thermistors, with an SPI interface.

Testing procedure

Output of the test application in both plain and SAUL modes at room temperature:

Initializing MAX31865 converter...      [OK]

Data:             24.72 °C
Data:             24.72 °C
Data:             24.70 °C
Data:             24.64 °C
Data:             24.67 °C
Data:             24.67 °C
Data:             24.67 °C
  • Values compared against a Testo 0560 1109 within ±0.2 °C.
  • Measured melting ice temperature at 400 m a.s.l to less than about +0.5 °C.

Issues/PRs references

Fixes #21362

@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: drivers Area: Device drivers Area: SAUL Area: Sensor/Actuator Uber Layer labels May 17, 2025
@crasbe crasbe added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 17, 2025
@riot-ci
Copy link
Copy Markdown

riot-ci commented May 17, 2025 •
edited
Loading

Murdock results

✔️ PASSED

49cb1a4 tests/drivers/max31865: test implementation

Success Failures Total Runtime
10499 0 10502 13m:59s

Artifacts

crasbe
crasbe reviewed May 17, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@crasbe crasbe left a comment

Choose a reason for hiding this comment

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

Thank you for this driver, the code looks quite good already. I have some small remarks and there are some static errors you could take a look at (either by running make static-test or in the Files tab here https://github.com/RIOT-OS/RIOT/pull/21498/files ).

I'll have to give it a more thorough look later (as in "a couple of days"), it's quite a chunk of code 😅

@dpproto dpproto force-pushed the 21362-add-driver-max31865 branch 2 times, most recently from c5d3e58 to d6f4085 Compare May 20, 2025 21:08
crasbe
crasbe reviewed May 21, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@crasbe crasbe left a comment

Choose a reason for hiding this comment

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

Some minor styling comments only.

@dpproto dpproto force-pushed the 21362-add-driver-max31865 branch from d6f4085 to 54e2ec7 Compare May 21, 2025 13:14
crasbe
crasbe reviewed May 21, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@crasbe crasbe left a comment

Choose a reason for hiding this comment

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

This is what I had in mind 😅

crasbe
crasbe reviewed May 21, 2025
View reviewed changes
@dpproto dpproto force-pushed the 21362-add-driver-max31865 branch from 54e2ec7 to 6346a30 Compare May 21, 2025 14:11
@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented May 21, 2025 •
edited
Loading

@dpproto May I ask you again to use git commit --fixup for the changes of already pushed commits during the review process instead of force-pushing squashed commits everytime. It makes the tracking of changes easier.

Please refer Add fixup commits during review and Squash commits after review.

gschorcht
gschorcht reviewed May 21, 2025
View reviewed changes
gschorcht
gschorcht requested changes May 21, 2025
View reviewed changes
@dpproto
Copy link
Copy Markdown
Contributor Author

dpproto commented May 22, 2025

@dpproto May I ask you again to use git commit --fixup for the changes of already pushed commits during the review process instead of force-pushing squashed commits everytime. It makes the tracking of changes easier.

Please refer Add fixup commits during review and Squash commits after review.

Sorry for the mess, and thanks for the link. I realize now that I should not use the --autosquash option. I'm not exactly a Git guru...

gschorcht
gschorcht requested changes May 23, 2025
View reviewed changes
@dpproto
Copy link
Copy Markdown
Contributor Author

dpproto commented May 23, 2025

@dpproto May I ask you again to use git commit --fixup for the changes of already pushed commits during the review process instead of force-pushing squashed commits everytime. It makes the tracking of changes easier.
Please refer Add fixup commits during review and Squash commits after review.

Should I make 1 commit for every conversation on GitHub?

@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented May 23, 2025

Should I make 1 commit for every conversation on GitHub?

No, you can bundle multiple changes as one fixup for the same commit.

gschorcht
gschorcht reviewed May 23, 2025
View reviewed changes
gschorcht
gschorcht reviewed May 23, 2025
View reviewed changes
gschorcht
gschorcht reviewed May 23, 2025
View reviewed changes
gschorcht
gschorcht reviewed May 23, 2025
View reviewed changes
gschorcht
gschorcht reviewed May 23, 2025
View reviewed changes
gschorcht
gschorcht reviewed May 23, 2025
View reviewed changes
@dpproto dpproto force-pushed the 21362-add-driver-max31865 branch from 6bf88c1 to 1938d5a Compare June 23, 2025 20:18
@dpproto
Copy link
Copy Markdown
Contributor Author

dpproto commented Jun 24, 2025

Please squash.

OK, it was easier than I thought 😄.

I just submitted a fixup commit to fix some uncrustify warnings.

@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Jun 24, 2025

I just submitted a fixup commit to fix some uncrustify warnings.

Please squash.

@dpproto dpproto force-pushed the 21362-add-driver-max31865 branch from 8a0f189 to f776910 Compare June 24, 2025 15:33
@gschorcht gschorcht enabled auto-merge June 24, 2025 15:38
@gschorcht gschorcht added this pull request to the merge queue Jun 24, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 24, 2025
@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Jun 24, 2025 •
edited
Loading

The test application needs a Makefile.ci for boards with insufficient RAM. Please take a look to such a Makefile.ci, e.g. tests/drivers/lis2dh12/Makefile.ci, to get a hint which boards could be affected and figure out which of them are really affected. Please blacklist only boards that are really affected.

@dpproto
Copy link
Copy Markdown
Contributor Author

dpproto commented Jun 25, 2025 •
edited
Loading

The test application needs a Makefile.ci for boards with insufficient RAM. Please take a look to such a Makefile.ci, e.g. tests/drivers/lis2dh12/Makefile.ci, to get a hint which boards could be affected and figure out which of them are really affected. Please blacklist only boards that are really affected.

Oops, I read and pushed too fast; I made a simple copy of tests/drivers/lis2dh12/Makefile.ci. How can I find the amount of RAM actually used by the test application? Is there a list somewhere of the amount of RAM for every board?

@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Jun 25, 2025

How can I find the amount of RAM actually used by the test application? Is there a list somewhere of the amount of RAM for every board?

size -A examples/basic/hello-world/bin/arduino-uno/hello-world.elf

section                      size      addr
.data                         110   8388864
.text                        5024         0
.bss                          898   8388974

@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Jun 25, 2025

Another tool is cosy

BOARD=arduino-uno make -j4 -C examples/basic/hello-world cosy

@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Jun 25, 2025 •
edited
Loading

You could also use target generate-Makefile.ci

make -C examples/basic/hello-world generate-Makefile.ci

but this will compile the application for all boards which takes a long time.

I would start with the boards defined in tests/drivers/lis2dh12/Makefile.ci and would compile them by hand to see whether it has to be added to your Makefile.ci.

crasbe
crasbe reviewed Jun 25, 2025
View reviewed changes
crasbe
crasbe reviewed Jun 26, 2025
View reviewed changes
@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Jun 26, 2025

@dpproto Please squash.

@dpproto
Copy link
Copy Markdown
Contributor Author

dpproto commented Jun 27, 2025

@dpproto Please squash.

I compile the test app for the Seeed Studio Xiao ESP32C3 board like this: USEMODULE+=stdio_usb_serial_jtag make -j12 clean all.

When I run size -A bin/seeedstudio-xiao-esp32c3/tests_max31865.elf, the sum of .debug_* lines make up about 90% of the total size. Is it expected?

@dpproto dpproto force-pushed the 21362-add-driver-max31865 branch from c31e4f8 to 49cb1a4 Compare June 27, 2025 07:28
@gschorcht
Copy link
Copy Markdown
Contributor

gschorcht commented Jun 27, 2025

When I run size -A bin/seeedstudio-xiao-esp32c3/tests_max31865.elf, the sum of .debug_* lines make up about 90% of the total size. Is it expected?

Don't worry about that, these debugging data are not part of the generated bin that is finally flashed. When you use size bin/seeedstudio-xiao-esp32c3/tests_max31865.elf you get the summary without debugging data.

@dpproto
Copy link
Copy Markdown
Contributor Author

dpproto commented Jun 27, 2025

@dpproto Please squash.

OK, done.

@gschorcht gschorcht added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 27, 2025
@gschorcht gschorcht added this pull request to the merge queue Jun 27, 2025
Merged via the queue into RIOT-OS:master with commit b562cba Jun 27, 2025
27 checks passed
@dpproto dpproto deleted the 21362-add-driver-max31865 branch June 30, 2025 09:34
@Teufelchen1 Teufelchen1 added this to the Release 2025.07 milestone Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crasbe crasbe crasbe left review comments

@gschorcht gschorcht gschorcht approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

Assignees

No one assigned

Labels

Area: doc Area: Documentation Area: drivers Area: Device drivers Area: SAUL Area: Sensor/Actuator Uber Layer Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Milestone

Release 2025.07

Development

Successfully merging this pull request may close these issues.

drivers/max31865: add support for RTD-to-digital converter

5 participants

@dpproto @riot-ci @gschorcht @crasbe @Teufelchen1