1-wire: scan does not work with multiple devices

[jbglaw]

I bought a HydraBus a few days ago and started to play with it:

> show system
HydraFW (HydraBus) v0.9-beta-115-ge463bc7 2020-01-29
sysTime: 0x046b48aa.
cyclecounter: 0x00809116 cycles.
cyclecounter64: 0x0000000000809128 cycles.
10ms delay: 1680032 cycles.

MCU Info
DBGMCU_IDCODE:0x10076413
CPUID:        0x410FC241
Flash UID:    0x31001A 0x58535011 0x20383658
Flash Size:   1024KB

Kernel:       ChibiOS 5.1.0
Compiler:     GCC 4.9.3 20150529 (release) [ARM/embedded-4_9-branch revision 227977]
Architecture: ARMv7E-M
Core Variant: Cortex-M4F
Port Info:    Advanced kernel mode
Platform:     STM32F405 High Performance with DSP and FPU
Board:        HydraBus 1.0
Build time:   Jan 29 2020 - 20:45:13

The firmware isn't all up-to-date, but I don't have the STM flasher available, so I probably cannot easily update it.

However, as the onewire code didn't change since 2019, I experience issues with the scan command, which are probably still valid.

• Attached are two (original) DS1820 (no "S" version or anything newer), to +5V, Data, GND.
• Communication works, but only after pull up. Maybe that would be a more sensible default than floating?
• The scan command works with one DS1820 attached:

> 1-wire
Device: onewire1
GPIO resistor: floating
Bit order: LSB first
onewire1> pull up
onewire1> scan
Discovered devices : 10 54 D0 D3 00 08 00 A8 
onewire1> 

• ...or the other:

onewire1> scan
Discovered devices : 10 D5 E9 D3 00 08 00 02 

• Attaching both DS1820 in parallel, it scans indefinitely until Reset/UBTN:

onewire1> scan
Discovered devices : 10 54 D0 D3 00 08 00 A8 
10 54 D0 D3 00 08 00 A8 
10 54 D0 D3 00 08 00 A8 
10 54 D0 D3 00 08 00 A8 
10 54 D0 D3 00 08 00 A8 
[...]

and only finds one of the two.

• Both DS1820 are working. I can (with only one attached) scan for both of them, and when both are attached, I can individually send them ROM_MATCH commands and let them sample a temperature or return their latest temperature reading.

I'd be willing to hack on this, but I would need to have a programmer, is that right? Flashing over USB or UART does not work?

[bvernoux]

1. For information the STM flasher is not required to update the firmware and you can flash HydraBus v1 firmware just with a micro USB cable connected on USB1 like described in following Wiki (for Windows or Linux)

   • https://github.com/hydrabus/hydrafw/wiki/Getting-Started-with-HydraBus-flash-and-use-hydrafw-on-windows
   • https://github.com/hydrabus/hydrafw/wiki/Getting-Started-with-HydraBus-flash-and-use-hydrafw-on-linux

2. For your issue about DS1820 scan when 2 are in parallel it shall be checked to understand what is wrong and to fix it (if such configuration is possible)

   • Thanks for your issue, it clearly seems we have a bug on scan command with that use case (multiple 1-wire devices reproduced with 2x DS1820 at same time) to be fixed
     • To be checked if our scan algorithm is fully compliant with "1-Wire Search Algorithm" from Maxim Integrated (Now part of Analog Devices) cf https://pdfserv.maximintegrated.com/en/an/AN187.pdf
     • Actual source code https://github.com/hydrabus/hydrafw/blob/master/src/hydrabus/hydrabus_mode_onewire.c#L183
   • On my side I do not have DS1820 but I will buy few to reproduce and fix that issue
   • If you have other 1-wire devices you are welcome to provide test and feedback to know if that issue can be reproduced especially with more than 1-wire device in parallel

[jbglaw]

I won't be able to look into it this weekend, but this is Maxim's description of the 1-wire rom address search algorithm: https://www.maximintegrated.com/en/design/technical-documents/app-notes/1/187.html

[jbglaw]

My impression is that onewire_scan() can only come up with one ROM address and there would be an outer driver needed to implement a tree search. It correctly detects that there's another device on the bus, but a caller would have to direct onewire_scan() to search for a different device. At least, that's what's happening in Maxim's application note and, as far as I now seem to understand the way the one-wire bus works, that's quite sensible.

[jbglaw]

Just tried a self-built compiler I had around (I also do buildrobot testing mass-builds of GCC and Binutils), but that's failing:

jbglaw@charon:~/src/gnu/hydrafw/src [master] $ make V=1
Creating ./common/hydrafw_version.hdr
Compiling crt0_v7m.S
arm-eabi-gcc: fatal error: cannot read spec file ‘nosys.specs’: No such file or directory
compilation terminated.
make: *** [chibios/os/common/startup/ARMCMx/compilers/GCC/rules.mk:253: build/obj/crt0_v7m.o] Error 1

So I fear I'll need to download the pre-built toolchain.

(NB: The Makefile still has python calls in it. Time to move forward to python3?)

[jbglaw]

Note to self: If I can come up with a toolchain based on all master HEADs, I'd also drop a HydraFW build to the automated build tests.

[bvernoux]

Just tried a self-built compiler I had around (I also do buildrobot testing mass-builds of GCC and Binutils), but that's failing:

jbglaw@charon:~/src/gnu/hydrafw/src [master] $ make V=1
Creating ./common/hydrafw_version.hdr
Compiling crt0_v7m.S
arm-eabi-gcc: fatal error: cannot read spec file ‘nosys.specs’: No such file or directory
compilation terminated.
make: *** [chibios/os/common/startup/ARMCMx/compilers/GCC/rules.mk:253: build/obj/crt0_v7m.o] Error 1

So I fear I'll need to download the pre-built toolchain.

Yes it is not recommended to build your own toolchain for ARM Embedded GCC and use a validated pre-built one like described in Wiki:

• https://github.com/hydrabus/hydrafw/wiki/how-to-build-flash-and-use-hydrafw-on-windows
• https://github.com/hydrabus/hydrafw/wiki/how-to-build-flash-and-use-hydrafw-on-linux

In theory we could also use GNU Arm Embedded Toolchain 8-2018-q4-major or more but I have never validated those toolchains (except through stm32cubeide) and I doubt they bring anything to the hydrafw project (and potentially could have regression like seen in the paste or issues until fully validated with hydrafw)

(NB: The Makefile still has python calls in it. Time to move forward to python3?)

About python dependency it is planned to be removed to build the firmware see #132
Also it works perfectly with python3 and in theory it is still compatible with the old deprecated python2 (but not supported anymore)

[bvernoux]

@jbglaw Tell me if you want to be assignee to fix that issue as so far I cannot find any DS1820 available and the DS18S20 are also out of stock and very expensive because of chip shortage.

• So far it is not clear if this issue can be reproduced only with 2x DS1820 (as I suspect it could be reproduced with any 1-wire chipset when using more than 1 chipset at a time and doing a scan)
• I have found some cheap TI BQ2026LPR available to be tested (but it will take time until I receive them and switch to that task) so you are welcome if you want to fix that issue

The actual code related to the scan shall be modified and better tested to be fully compliant with 1-Wire Search Algorithm

[jbglaw]

At least, I just managed to flash my HydraBus with a self-build (using the provided toolchain) firmware image:

> show system
HydraFW (HydraBus) v0.10-50-gc2a43c3-dirty 2022-06-26
sysTime: 0x0002ae8e.
cyclecounter: 0xafff2e44 cycles.
cyclecounter64: 0x00000000afff2e53 cycles.
10ms delay: 1680029 cycles.

MCU Info
DBGMCU_IDCODE:0x10076413
CPUID:        0x410FC241
Flash UID:    0x31001A 0x58535011 0x20383658
Flash Size:   1024KB

Kernel:       ChibiOS 5.1.0
Compiler:     GCC 4.9.3 20150529 (release) [ARM/embedded-4_9-branch revision 227977]
Architecture: ARMv7E-M
Core Variant: Cortex-M4F
Port Info:    Advanced kernel mode
Platform:     STM32F405 High Performance with DSP and FPU
Board:        HydraBus 1.0
Build time:   Sep 24 2022 - 21:59:26

I'm quite sure the scan issue will show up with any 1-wire device, it's just that I had the DS1820 flyin' around from some old projects. Indeed, that very IC is quite superceded by newer versions of that design. As I'm now set-up to build and test the firmware, I'll try to have a look at this. (Though don't expect results for the next few days, multiple birthdays coming up.)

[jbglaw]

Well... I ported the sample code:

> 1-wire
Device: onewire1
GPIO resistor: floating
Bit order: LSB first
onewire1> pull up
onewire1> scan
Scanning bus for devices.
1: 10 54 D0 D3 00 08 00 A8 
2: 10 D5 E9 D3 00 08 00 02 
onewire1> 

I'll attach my current patch, but I don't consider this RFP right now:

• It follows my coding style.
• Search state should go into a separate struct and passed through instead of relying on global variables.
• Works for me[tm].
• I'd like to store the found "MAC" addresses and be able to reference them like such an example (this is made up, not yet coded):

> 1-wire
Device: onewire1
GPIO resistor: floating
Bit order: LSB first
onewire1> pull up
onewire1> scan
Scanning bus for devices.
1: 10 54 D0 D3 00 08 00 A8    <--------------
2: 10 D5 E9 D3 00 08 00 02 

# Let Sensor 1 sample temperature, giving the MAC address explicitely:
onewire1> [ 0x55 0x10 0x54 0xD0 0xD3 0x00 0x08 0x00 0xA8 0x44
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
# Let Sensor 1 sample temperature, use discovered address:
onewire1> [ 0x55 <1> 0x44

onewire-scan.patch.txt

[bvernoux]

For the "macro" management (with <1> ...) please create an other issue to implement that in the firmware as so far that concept does not exist

[jbglaw]

I opened an issue for getting infrastructure for scan results into place.

Meanwhile, here's an updated patch. The search state is placed into a struct (instead of global variables). Works for me. I'd appreciate comments on this new patch: onewire-scan.patch.txt.

[bvernoux]

Thanks for your contribution the onewire-scan.patch.txt seems good could you do a PR for final review/merge ?

[jbglaw]

I will prepare one. That'll also do a final rename of some variables (those still use ugly CamelCase) and I'll prepare two branches: One for just the scan change, and the second for the sneaked-in s/python/python3/ change in the main Makefile. The existing Python scripts seem to Just Work[tm] when being executed by python3.

[bvernoux]

Thanks for the PR #139

[Baldanos]

Fixed by #139.
Thanks a lot @jbglaw !