Draft: Add 1-Wire driver #19848

Enoch247 · 2023-07-26T19:22:42Z

Contribution description

This patch adds a 1-Wire driver which can handle multiple driver implementations. Included in the patch is a soft 1-Wire driver implementation.

Testing procedure

TODO

Issues/PRs references

PR DRAFT: Add onewire driver #18051 was an earlier attempt of mine.
PR drivers: add generic 1-Wire bus driver #14897 was an attempt that predated my first attempt. I used it as a starting point this time. Although, not much of it actually remains.

dylad · 2023-07-27T07:45:32Z

Nice PR !
I don't think we need to define a test for the high-level API. IMO, it will be good enough to glue it to some low level driver and ensure the hardware works as expected.
Can I test this PR with a DS18B20 probe ?

Enoch247 · 2023-07-27T13:13:15Z

I don't think we need to define a test for the high-level API. IMO, it will be good enough to glue it to some low level driver and ensure the hardware works as expected.

Ok. I don't know much about RIOT's test system yet. I will work on getting up to speed and write some tests for this PR.

Can I test this PR with a DS18B20 probe ?

The onewire_search() and onewire_read_rom() should work on a ds18b20 device. I plan to convert the ds18 driver over to use this new API, but haven't yet (that will be a different PR probably). Once that's done then you'll be able to read the device's temperature using the ds18 driver via this new onewire API.

The way I've been testing this API and driver so far is with a ds2433 and driver. The ds2433 driver is functional, but still needs work before I submit it for PR.

drivers/include/onewire.h

crasbe · 2025-04-05T11:51:58Z

drivers/include/onewire.h

+/**
+ * @brief   Search for devices on the given 1-Wire bus
+ *
+ * @see     https://pdfserv.maximintegrated.com/en/an/AN937.pdf page 51+52


This link now redirects to https://www.analog.com/media/en/technical-documentation/tech-articles/book-of-ibuttonreg-standards.pdf

drivers/include/onewire.h

drivers/include/soft_onewire.h

tests/driver_onewire/main.c

crasbe · 2025-04-05T12:04:42Z

drivers/onewire/onewire.c

+    }
+
+    /* Start search. Refer to e.g. to
+     * https://pdfserv.maximintegrated.com/en/an/AN937.pdf, page 51 and 52 for


Same as above, the URL redirects to a new page now.

crasbe · 2025-04-05T12:06:57Z

drivers/soft_onewire/soft_onewire.c

+//TODO: use bitfield.h for tx/rx buffers?
+
+/* bus timings as given in
+ * https://www.maximintegrated.com/en/design/technical-documents/app-notes/1/126.html


This redirects to https://www.analog.com/en/resources/technical-articles/1wire-communication-through-software.html now.

crasbe · 2025-04-05T12:08:09Z

drivers/soft_onewire/soft_onewire.c

+ //TODO: better names?
+#define T_RESET_HOLD_US     (480U)
+#define T_RESET_SAMPLE_US   (70U)
+#define T_RESET_RECOVERY_US (410U)
+#define T_RW_START_US       (6U)
+#define T_W_0_HOLD_US       (60U)
+#define T_W_0_END_US        (10U)
+#define T_W_1_DELAY_US      (64U)
+#define T_R_WAIT_US         (9U)
+#define T_R_END_US          (55U)


Maybe something like "ONEWIRE_RESET_HOLD_US"? With the "US" it should be quite clear that it's a time.

tests/driver_onewire/main.c

crasbe · 2025-04-23T12:38:00Z

I think the test would need some more instructions, in a README.md file or so. I wasn't really sure how to get it working three weeks ago and wasn't successful in doing so 😅

Enoch247 · 2025-04-24T01:41:26Z

Yeah, the test is not really complete as is. After writing the test, I realized what a pain it is to define a bus and its params. So I'm reworking that now. I also went ahead and ported the ds18 driver to use the onewire API (coming in a separate PR). I've re-worked the API to use less RAM and ROM when only one back-end is enabled too. You should see some changes in this PR once I am finished. Pretty close...

Enoch247 · 2025-04-28T01:43:16Z

At this point. I don't expect to make any more big changes to the driver code. The test code still needs some work and doc. I'm also scratching my head on how the test could be made to be common as it really could be used to test any onewire backend. I hate the thought of each new backend just copying the test and have a bunch of near duplicates.

Enoch247 · 2025-04-28T01:51:40Z

Can I test this PR with a DS18B20 probe ?

@dylad : a few years later, yes. See my WIP branch for that.

Enoch247 · 2025-04-29T02:56:41Z

drivers/include/onewire.h

+static inline uint_fast8_t onewire_crc8(uint_fast16_t seed, const void *data,
+    size_t len)
+{
+    return crc8(data, len, 0x80, seed);


Hmm, I think I am not calculating the CRC properly here. Still working on figuring out the right polynomial and how to handle reflecting the input.

One thing up front: the polynomial is wrong. CRC8 for 1-Wire uses 0x8C and not 0x80. Actually 0x80 won't generate good CRC checksums anyways.

So it depends on if you really want to use the built-in CRC8 function or not.

For the built-in function to work, you have to reflect all the input bytes, the polynomial and the output byte. That is a lot of computation.

I wrote a smöl test program to demonstrate:

#include <checksum/crc8.h> #include <stdio.h> // from: https://github.com/nickandrew/avr-onewire/blob/master/maxim-crc8.c uint8_t crc8_update(uint8_t crc, uint8_t data) { uint8_t i; crc = crc ^ data; for (i = 0; i < 8; ++i) { if (crc & 0x01) { crc = (crc >> 1) ^ 0x8c; } else { crc >>= 1; } } return crc; } uint8_t reflect_byte(uint8_t b) { b = (b & 0xF0) >> 4 | (b & 0x0F) << 4; b = (b & 0xCC) >> 2 | (b & 0x33) << 2; b = (b & 0xAA) >> 1 | (b & 0x55) << 1; return b; } int main(void) { for(uint32_t i = 0; i < 256; i++) { uint8_t i_refl = reflect_byte(i); uint8_t riot_CRC8 = reflect_byte(crc8((uint8_t*) &i_refl, 1, 0x31, 0x00)); uint8_t onewire_CRC8 = crc8_update(0, (uint8_t) i); uint8_t xor_CRC8 = riot_CRC8 ^ onewire_CRC8; printf("i: %03d, CRC8: %02X, Onewire CRC8: %02X, XOR: %02X\n",i, riot_CRC8, onewire_CRC8, xor_CRC8); } return 0; }

And the output is the following:

2025-06-06 15:34:59,666 # RIOT native hardware initialization complete. 2025-06-06 15:34:59,667 # 2025-06-06 15:34:59,668 # main(): This is RIOT! (Version: 2025.04-devel-500-g42e08-add-driver-onewire) 2025-06-06 15:34:59,668 # i: 000, CRC8: 00, Onewire CRC8: 00, XOR: 00 2025-06-06 15:34:59,669 # i: 001, CRC8: 5E, Onewire CRC8: 5E, XOR: 00 2025-06-06 15:34:59,669 # i: 002, CRC8: BC, Onewire CRC8: BC, XOR: 00 2025-06-06 15:34:59,669 # i: 003, CRC8: E2, Onewire CRC8: E2, XOR: 00 2025-06-06 15:34:59,670 # i: 004, CRC8: 61, Onewire CRC8: 61, XOR: 00 2025-06-06 15:34:59,671 # i: 005, CRC8: 3F, Onewire CRC8: 3F, XOR: 00 2025-06-06 15:34:59,671 # i: 006, CRC8: DD, Onewire CRC8: DD, XOR: 00 2025-06-06 15:34:59,671 # i: 007, CRC8: 83, Onewire CRC8: 83, XOR: 00 2025-06-06 15:34:59,672 # i: 008, CRC8: C2, Onewire CRC8: C2, XOR: 00 2025-06-06 15:34:59,672 # i: 009, CRC8: 9C, Onewire CRC8: 9C, XOR: 00 2025-06-06 15:34:59,673 # i: 010, CRC8: 7E, Onewire CRC8: 7E, XOR: 00 2025-06-06 15:34:59,673 # i: 011, CRC8: 20, Onewire CRC8: 20, XOR: 00 2025-06-06 15:34:59,674 # i: 012, CRC8: A3, Onewire CRC8: A3, XOR: 00 2025-06-06 15:34:59,675 # i: 013, CRC8: FD, Onewire CRC8: FD, XOR: 00 2025-06-06 15:34:59,675 # i: 014, CRC8: 1F, Onewire CRC8: 1F, XOR: 00 2025-06-06 15:34:59,676 # i: 015, CRC8: 41, Onewire CRC8: 41, XOR: 00 2025-06-06 15:34:59,676 # i: 016, CRC8: 9D, Onewire CRC8: 9D, XOR: 00 2025-06-06 15:34:59,677 # i: 017, CRC8: C3, Onewire CRC8: C3, XOR: 00 2025-06-06 15:34:59,678 # i: 018, CRC8: 21, Onewire CRC8: 21, XOR: 00 2025-06-06 15:34:59,679 # i: 019, CRC8: 7F, Onewire CRC8: 7F, XOR: 00 2025-06-06 15:34:59,679 # i: 020, CRC8: FC, Onewire CRC8: FC, XOR: 00 2025-06-06 15:34:59,680 # i: 021, CRC8: A2, Onewire CRC8: A2, XOR: 00 ...

IMO it would be best to just add a crc8r or crc8_refl or crc8_reflected function to sys/checksum/crc8 and extend the unittest for it.
Perhaps in a separate PR.

Maybe I got some time to look at that.

Suggested change

return crc8(data, len, 0x80, seed);

return crc8_lsb(data, len, 0x8C, seed);

Once #21543 is merged, you can change it to this. Remember to also change the CRC polynomial.

crasbe · 2025-05-07T13:11:20Z

Unfortunately I don't have the Discovery at hand, but I tried with an nRF52840DK and a Nucleo-L152RE and L073RZ.

The nRF52840DK just doesn't work, it does not recognize the DS18B20. Neither with the tests/drivers/onewire test (after I added an unconditional .pin = GPIO_PIN(0,3), to the struct) nor with the tests/drivers/ds18 (same modification).

This is the log (with ENABLE_DEBUG = 1 in the drivers/onewire/onewire.c):

2025-05-07 14:37:29,551 # main(): This is RIOT! (Version: 2025.04-devel-500-g42e08-add-driver-onewire)
2025-05-07 14:37:29,552 # _onewire_init
2025-05-07 14:37:29,555 # onewire_write: f0
2025-05-07 14:37:29,578 # found device: 0000000000000000
2025-05-07 14:37:29,581 # onewire_write: f0
2025-05-07 14:37:29,605 # failure to enumerate device: EBADMSG

The same happens on other GPIO pins as well and also with an external 4.7k Pull Up.

It appears like some data is exchanged on the bus, but idk enough about OneWire to tell what might be the fault here.
Without external Pullup:

With Pullup:

For good measure I tried with an 1k Pullup, but that leads to the same result.

The sensor is a fake, but it does work. I checked it with an Arduino Uno and this tool: https://github.com/cpetrich/counterfeit_DS18B20

Sensor ROM and Current Scratchpad Content:
   1. 28-54-DE-15-C0-21-08-3D, 28-0821C015DE54: 50/05/4B/46/7F/FF/0C/10/1C 85.00 oC
  Number of Sensors: 1.

The Nucleos crash with a failed assertion:

2025-05-07 14:33:03,551 # main(): This is RIOT! (Version: 2025.04-devel-500-g42e08-add-driver-onewire)
2025-05-07 14:33:03,554 # 0x8001537 => FAILED ASSERTION.

cbuec@W11nMate:~/RIOTstuff/riot-onewire/RIOT/tests/drivers/onewire$ arm-none-eabi-objdump -d --start-address 0x8001520 --stop-address 0x8001564 bin/nucleo-l073rz/tests_onewire.elf

bin/nucleo-l073rz/tests_onewire.elf:     file format elf32-littlearm


Disassembly of section .text:

08001520 <_onewire_write_bits+0x44>:
 8001520:       08001341        .word   0x08001341
 8001524:       0800135d        .word   0x0800135d

08001528 <soft_onewire_init>:
 8001528:       b570            push    {r4, r5, r6, lr}
 800152a:       0004            movs    r4, r0
 800152c:       000d            movs    r5, r1
 800152e:       2800            cmp     r0, #0
 8001530:       d101            bne.n   8001536 <soft_onewire_init+0xe>
 8001532:       f7ff f83d       bl      80005b0 <_assert_panic>
 8001536:       2900            cmp     r1, #0
 8001538:       d0fb            beq.n   8001532 <soft_onewire_init+0xa>
 800153a:       f7ff fbe1       bl      8000d00 <_onewire_init>
 800153e:       2301            movs    r3, #1
 8001540:       425b            negs    r3, r3
 8001542:       60a3            str     r3, [r4, #8]
 8001544:       4a05            ldr     r2, [pc, #20]   ; (800155c <soft_onewire_init+0x34>)
 8001546:       0023            movs    r3, r4
 8001548:       4905            ldr     r1, [pc, #20]   ; (8001560 <soft_onewire_init+0x38>)
 800154a:       68a8            ldr     r0, [r5, #8]
 800154c:       f7ff fd2a       bl      8000fa4 <timer_init>
 8001550:       2800            cmp     r0, #0
 8001552:       d1ee            bne.n   8001532 <soft_onewire_init+0xa>
 8001554:       6820            ldr     r0, [r4, #0]
 8001556:       f7ff fe89       bl      800126c <_bus_release.isra.0>
 800155a:       bd70            pop     {r4, r5, r6, pc}
 800155c:       08001265        .word   0x08001265
 8001560:       000f4240        .word   0x000f4240

So it seems like it is the first assertion in the soft_onewire_init function.

riot-ci · 2025-06-06T13:48:00Z

Murdock results

❌ FAILED

42e088c WIP: drivers/soft_onewire: add tests

Success	Failures	Total	Runtime
4569	0	9811	05m:30s

Artifacts

crasbe

Some static errors about the documentation.

crasbe · 2025-06-06T13:48:47Z

drivers/include/onewire.h

+ * When called, the bus driver should perform a bus reset and detect the slave
+ * device present pulse.
+ *
+ * @param[in] bus       1-Wire bus descriptor


Suggested change

* @param[in] bus 1-Wire bus descriptor

* @param[in] super 1-Wire bus descriptor

crasbe · 2025-06-06T13:48:58Z

drivers/include/onewire.h

+/**
+ * @brief   Callback to transfer bytes from the bus
+ *
+ * @param[in] bus       1-Wire bus descriptor


Suggested change

* @param[in] bus 1-Wire bus descriptor

* @param[in] super 1-Wire bus descriptor

crasbe · 2025-06-06T13:49:18Z

drivers/include/onewire.h

+/**
+ * @brief   Callback to transfer bytes to the bus
+ *
+ * @param[in] bus       1-Wire bus descriptor


Suggested change

* @param[in] bus 1-Wire bus descriptor

* @param[in] super 1-Wire bus descriptor

crasbe · 2025-06-06T13:49:32Z

drivers/include/soft_onewire.h

+/**
+ * @brief Initialize soft 1-Wire bus
+ *
+ * @param[in] bus       bus descriptor


Suggested change

* @param[in] bus bus descriptor

* @param[in] super bus descriptor

crasbe · 2025-06-06T13:49:42Z

drivers/onewire/doc.md

+@defgroup   drivers_onewire_devs 1-Wire Device Drivers
+@ingroup    drivers_onewire
+@brief      1-Wire Slave Device Drivers
+


Suggested change

Enoch247 requested review from smlng, leandrolanzieri, aabadie, MichelRottleuthner and fjmolinas as code owners July 26, 2023 19:22

github-actions bot added Area: tests Area: tests and testing framework Area: build system Area: Build system Area: drivers Area: Device drivers labels Jul 26, 2023

Enoch247 changed the title ~~Add 1-Wire driver~~ Draft: Add 1-Wire driver Jul 26, 2023

Enoch247 force-pushed the add-driver-onewire branch from 9b16584 to cd75a9b Compare July 26, 2023 20:17

crasbe reviewed Apr 5, 2025

View reviewed changes

crasbe added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Apr 5, 2025

Enoch247 marked this pull request as draft April 10, 2025 00:02

Enoch247 force-pushed the add-driver-onewire branch from d92b70d to 39a6876 Compare April 10, 2025 01:40

Enoch247 force-pushed the add-driver-onewire branch from 39a6876 to a8dd266 Compare April 19, 2025 11:23

Enoch247 and others added 3 commits April 27, 2025 19:30

drivers/onewire: add onewire driver

cfd65c5

drivers/soft_onewire: add soft onewire driver

c09eee1

WIP: drivers/soft_onewire: add tests

42e088c

Enoch247 force-pushed the add-driver-onewire branch from a8dd266 to 42e088c Compare April 28, 2025 01:37

github-actions bot added Area: doc Area: Documentation and removed Area: build system Area: Build system labels Apr 28, 2025

Enoch247 added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Area: build system Area: Build system and removed Area: doc Area: Documentation labels Apr 28, 2025

Enoch247 commented Apr 29, 2025

View reviewed changes

crasbe added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 6, 2025

crasbe reviewed Jun 6, 2025

View reviewed changes

crasbe mentioned this pull request Jun 6, 2025

sys/checksum: Add LSB-first (reflected) CRC8 Function #21543

Merged

	return crc8(data, len, 0x80, seed);
	return crc8_lsb(data, len, 0x8C, seed);

	* @param[in] bus 1-Wire bus descriptor
	* @param[in] super 1-Wire bus descriptor

	* @param[in] bus bus descriptor
	* @param[in] super bus descriptor

Draft: Add 1-Wire driver #19848

Are you sure you want to change the base?

Draft: Add 1-Wire driver #19848

Conversation

Enoch247 commented Jul 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

dylad commented Jul 27, 2023

Uh oh!

Enoch247 commented Jul 27, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crasbe commented Apr 23, 2025

Uh oh!

Enoch247 commented Apr 24, 2025

Uh oh!

Enoch247 commented Apr 28, 2025

Uh oh!

Enoch247 commented Apr 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crasbe commented May 7, 2025

Uh oh!

riot-ci commented Jun 6, 2025

Murdock results

Artifacts

Uh oh!

crasbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Enoch247 commented Jul 26, 2023 •

edited

Loading