first implementation of 1-wire protocol #505

conejoninja · 2023-01-10T18:04:31Z

I would like @dn-kolesnikov to review this PR as it was mainly his work, I just adapted it to the proper repo structure.

Once merge, issue #489 could be closed.

deadprogram · 2023-01-21T09:24:33Z

Was just looking at this PR. So first of all seems to me that it would be better to structure the files like this:

onewire/onewire.go

And then separately

ds18b20/ds18b20.go

This seems more in keeping with the way things like lora and net are currently.

Future onewire devices (EEPROMs, fuel gauges, etc.) would be added at the root level, like pretty much all of the other drivers are now.

What do you think?

deadprogram · 2023-01-21T09:30:29Z

Also, to be in fully correct naming it should be 1-wire not onewire.

Due to Go naming rules this would turn into just wire, correct?

This seems appealing:

package somedevice

import "tinygo.org/x/drivers/1-wire"

func (d *Driver) DoSomething() {
    wire.Reset()
    wire.Write(...)
}

What do you think?

conejoninja · 2023-01-21T09:38:34Z

I just follow @dn-kolesnikov naming.

NOTE: There's a popular Arduino library called Wire that handles I²C communication, https://www.arduino.cc/reference/en/language/functions/communication/wire/ It might be confusing?

deadprogram · 2023-01-21T11:59:15Z

The name of the package here would be 1-wire In any case, I doubt there would be confusion over that particular name in the code.

Regarding the directory organization, I am not really in favor of the proposed structure. How do we proceed?

conejoninja · 2023-01-21T12:13:41Z

the structure you proposed seems ok to me and make sense. I'll make the changes and uodate the PR

conejoninja · 2023-01-21T16:22:53Z

I updated the package as suggested, renamed the folder to 1-wire , and the package to wire (suggested by the IDE was __wire), then, we are forced to import it as

import (
	"machine"
	"time"
	wire "tinygo.org/x/drivers/1-wire"

	"tinygo.org/x/drivers/ds18b20"
)

It's ok for me.

dn-kolesnikov · 2023-01-21T19:26:39Z

Was just looking at this PR. So first of all seems to me that it would be better to structure the files like this:

onewire/onewire.go

And then separately

ds18b20/ds18b20.go

This seems more in keeping with the way things like lora and net are currently.

Future onewire devices (EEPROMs, fuel gauges, etc.) would be added at the root level, like pretty much all of the other drivers are now.

What do you think?

The Arduino standard library is called OneWire (https://www.arduino.cc/reference/en/libraries/onewire/).

deadprogram · 2023-01-26T12:45:02Z

@dn-kolesnikov has a good point. We should probably name it onewire.

deadprogram · 2023-01-26T19:21:17Z

examples/ds18b20/main.go

+
+// SliceToHexString converts a slice to Hex string
+// fmt.Printf - compile error on an Arduino Uno boards
+func SliceToHexString(rom []uint8) string {


Just out of curiosity, what was the reason to not use encoding/hex?

Just forgot about it...

matveynator · 2023-01-26T19:45:58Z

Good day ). As each device are using tiny bits of onewire protocol implementation (as I understood - small memory usage wise) it is good that devices are added root level. "onewire" name also seems reasonable... I am also ready to help )

dn-kolesnikov · 2023-01-28T08:32:32Z

at the moment I am checking the function of searching for multiple devices on the bus. I had to change the Device structure and add a few additional functions. the next commit to my repository will break compatibility with the current versions of the library. the examples will also be rewritten taking into account the new changes.

conejoninja · 2023-01-28T09:13:09Z

at the moment I am checking the function of searching for multiple devices on the bus. I had to change the Device structure and add a few additional functions. the next commit to my repository will break compatibility with the current versions of the library. the examples will also be rewritten taking into account the new changes.

Feel free to make the PR directly from your repository. I did it because I needed for a FOSDEM talk, but I started to think it will not be merged on time, no need to rush.

deadprogram · 2023-01-28T10:16:00Z

I was actually getting ready to merge this unless there is some reason I shouldn't?

conejoninja · 2023-01-28T10:18:20Z

I was actually getting ready to merge this unless there is some reason I shouldn't?

well, @dn-kolesnikov commented the next change will break the compatibility, so I guess it's better to wait

bgould · 2023-03-07T02:49:48Z

@conejoninja Sounds like @dn-kolesnikov has some new version of this mentioned in #489 ... how should we move forward with this, do you want to merge the changes into your branch or just should @dn-kolesnikov start a new one?

bxparks · 2023-04-04T15:43:14Z

onewire/onewire.go

+func (d Device) WriteBit(data uint8) {
+	d.p.Configure(machine.PinConfig{Mode: machine.PinOutput})
+	if data&1 == 1 { // Send '1'
+		time.Sleep(5 * time.Microsecond)


Sorry for the late comment, but I wasn't around in January when this PR was started.

I just want to point out that time.Sleep() yields to the goroutine scheduler. There is no guarantee that this will sleep for 5 microseconds. It could sleep for 100s of microseconds, or several milliseconds.

Since TinyGo does not provide a non-yielding runtime.delayMicros() or machine.delayMicros() function, as far as I know, maybe there is no other option right now.

I have conducted several experiments for the chips I have (rp2040, stm32f103, stm32f411, attiny85, atmega328p). Created a project:

package main import "time" func main() { for { time.Sleep(5 * time.Microsecond) } }

Got an assembler listing and studied them. In all variants, a similar tick counting mechanism was used to organize the delay.

That's interesting.. Have you looked at the SAMD21? I've been struggling with it. time.Sleep() seems to have an overhead of 250-400 microseconds. The following program prints: elapsed millis: 36621 instead of a number close to 500 milliseconds:

package main import "time" func main() { time.Sleep(2 * time.Second) // wait for serial monitor println("Starting...") start := time.Now() for i := 0; i < 100_000; i++ { time.Sleep(5 * time.Microsecond) } elapsed := time.Since(start) println("elapsed millis:", elapsed.Milliseconds()) }

bxparks · 2023-04-04T15:49:09Z

onewire/onewire.go

+	// Dow-CRC using polynomial X^8 + X^5 + X^4 + X^0
+	// Tiny 2x16 entry CRC table created by Arjen Lentz
+	// See http://lentz.com.au/blog/calculating-crc-with-a-tiny-32-entry-lookup-table
+	crc8_table := [...]uint8{


Just curious, is the TinyGo compiler smart enough to store this as a constant in flash, or will it create this on the stack? I guess it's only 32 bytes, so it doesn't matter except maybe on AVR processors. In my experience, a const string was the only way I could guarantee that an array like this would be stored in flash.

The smallest of the AVR ATtiny85 microcontrollers supported in Tinygo has 512 bytes of RAM. If 32 bytes are critical for your project, you can use a fast CRC calculation algorithm for avr in assembler. Or use
const crc8_table string = "\x00\x5E\xBC\xE2\x61\x3F\xDD\x83\xC2\x9C\x7E\x20\xA3\xFD\x1F\x41\x00\x9D\x23\xBE\x46\xDB\x65\xF8\x8C\x11\xAF\x32\xCA\x57\xE9\x74"

conejoninja · 2023-04-09T05:46:40Z

I think the conversation is going a bit off-topic. We could merge this and make improvements later.

bxparks · 2023-04-09T15:05:28Z

I think the conversation is going a bit off-topic. We could merge this and make improvements later.

Sorry about that, I agree.

deadprogram · 2023-04-22T15:54:03Z

OK based on the last comments I am squash/merging. Thank you very much everyone who worked on this!

first implementation of 1-wire protocol

3aafe42

conejoninja requested a review from deadprogram January 10, 2023 18:04

conejoninja and others added 3 commits January 10, 2023 19:06

Merge branch 'dev' into onewire

47f3cac

drivers updates

8e45982

Merge branch 'onewire' of github.com:conejoninja/drivers into onewire

a4a53f7

conejoninja and others added 2 commits January 21, 2023 17:18

moved files, renamed package

a93f86a

Merge branch 'dev' into onewire

55eb954

replace const too

8bd3194

one last name change

c07bd25

deadprogram reviewed Jan 26, 2023

View reviewed changes

updated to last version

0459a48

dn-kolesnikov approved these changes Apr 4, 2023

View reviewed changes

bxparks reviewed Apr 4, 2023

View reviewed changes

conejoninja requested a review from deadprogram April 9, 2023 05:44

deadprogram merged commit 5cc2132 into tinygo-org:dev Apr 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

first implementation of 1-wire protocol #505

first implementation of 1-wire protocol #505

conejoninja commented Jan 10, 2023

deadprogram commented Jan 21, 2023

deadprogram commented Jan 21, 2023

conejoninja commented Jan 21, 2023

deadprogram commented Jan 21, 2023

conejoninja commented Jan 21, 2023

conejoninja commented Jan 21, 2023 •

edited

Loading

dn-kolesnikov commented Jan 21, 2023

deadprogram commented Jan 26, 2023

deadprogram Jan 26, 2023

dn-kolesnikov Jan 28, 2023

matveynator commented Jan 26, 2023

dn-kolesnikov commented Jan 28, 2023

conejoninja commented Jan 28, 2023

deadprogram commented Jan 28, 2023

conejoninja commented Jan 28, 2023

bgould commented Mar 7, 2023

bxparks Apr 4, 2023

dn-kolesnikov Apr 6, 2023

bxparks Apr 6, 2023

bxparks Apr 4, 2023

dn-kolesnikov Apr 6, 2023

conejoninja commented Apr 9, 2023

bxparks commented Apr 9, 2023

deadprogram commented Apr 22, 2023

first implementation of 1-wire protocol #505

first implementation of 1-wire protocol #505

Conversation

conejoninja commented Jan 10, 2023

deadprogram commented Jan 21, 2023

deadprogram commented Jan 21, 2023

conejoninja commented Jan 21, 2023

deadprogram commented Jan 21, 2023

conejoninja commented Jan 21, 2023

conejoninja commented Jan 21, 2023 • edited Loading

dn-kolesnikov commented Jan 21, 2023

deadprogram commented Jan 26, 2023

deadprogram Jan 26, 2023

Choose a reason for hiding this comment

dn-kolesnikov Jan 28, 2023

Choose a reason for hiding this comment

matveynator commented Jan 26, 2023

dn-kolesnikov commented Jan 28, 2023

conejoninja commented Jan 28, 2023

deadprogram commented Jan 28, 2023

conejoninja commented Jan 28, 2023

bgould commented Mar 7, 2023

bxparks Apr 4, 2023

Choose a reason for hiding this comment

dn-kolesnikov Apr 6, 2023

Choose a reason for hiding this comment

bxparks Apr 6, 2023

Choose a reason for hiding this comment

bxparks Apr 4, 2023

Choose a reason for hiding this comment

dn-kolesnikov Apr 6, 2023

Choose a reason for hiding this comment

conejoninja commented Apr 9, 2023

bxparks commented Apr 9, 2023

deadprogram commented Apr 22, 2023

conejoninja commented Jan 21, 2023 •

edited

Loading