2

I have a type that contains a byte of data, and takes a channel to post new data there. Other code can read the last written byte of data using a Read function.

Edit: for actual, runnable code, see https://github.com/ariejan/i6502/pull/3 especially files acia6551.go and acia6551_test.go. Tests results can be viewed here: https://travis-ci.org/ariejan/i6502/jobs/32862705

I have the following:

// Emulates a serial interface chip of some kind.
type Unit struct {
  // Channel used for others to use, bytes written here will be placed in rxChar
  Rx chan byte

  // Internal store of the last byte written.
  rxChar byte // Internal storage
}

// Used internally to read data store in rxChar
func (u *Unit) Read() byte {
  return u.rxChar
}

// Create new Unit and go-routing to listen for Rx bytes
func NewUnit(rx chan byte) *Unit {
  unit := &Unit{Rx: rx}

  go func() {
    for {
      select {
      case data := <-unit.Rx:
        unit.rxData = data
        fmt.Printf("Posted 0x%02X\n", data)
      }
    }
  }()

  return unit
}

My test looks like this:

func TestUnitRx(t *testing.T) {
  rx := make(chan byte)
  u := NewUnit(rx)

  // Post a byte to the Rx channel
  // This prints "Posted 0x42", as you'd expect
  rx <- 0x42

  // Using testing
  // Should read last byte, 0x42 but fails.
  fmt.Println("Reading value...")
  assert.Equal(t, 0x42, u.Read()) 
}

At first I figured the "Reading value" happened before the go-routing got around to writing the data. But the "Posted" message is always printed before "Reading".

So, two questions remain:

  • Is this the best way to handle an incoming stream of bytes (at 9600 baud ;-))
  • If this is the right way, how can I properly test it or what is wrong with my code?
Ariejan
  • 10,910
  • 6
  • 43
  • 40
  • Please fix your code. You're missing a closing bracket, and rxChar is undeclared in your Read method. Is rxData supposed to be rxChar? – JimB Aug 18 '14 at 14:21
  • It would help if you posted something that can be run. There's no way to know what happens between rxData and rxChar. – JimB Aug 18 '14 at 14:29
  • I've added a link to github for more details. This contains runnable code and the failing test. – Ariejan Aug 18 '14 at 14:41
  • 1
    So much code in that goroutine. It is approximately equivalent to `go func() { for unit.rxData = range unit.Rx { fmt.Printf("Posted 0x%02X\n", data) }` (except this will terminate properly if you close the channel) – Dustin Aug 18 '14 at 18:11
  • 2
    I +1 @Dustin. Also in your github commits. I would suggest to not use channels at all in your implementation. Just create a class that implements io.Readable out of all those serial interfaces. – fabrizioM Aug 18 '14 at 18:14
  • @fabrizioM classes don't exist in golang – KMA Badshah Apr 07 '21 at 08:44

1 Answers1

1

Guessing by the pieces posted here, it doesn't look like you have anything guaranteeing the order of operations when accessing the stored data. You can use a mutex around any data shared between goroutines.

A better option here is to use buffered channels of length 1 to write, store, and read the bytes.

It's always a good idea to test your program with -race to use the race detector.

Since this looks very "stream" like, you very well may want some buffering, and to look at some examples of how the io.Reader and io.Writer interfaces are often used.

JimB
  • 104,193
  • 13
  • 262
  • 255
  • True, that's what I tried to describe in my question. But how do I do achieve that? – Ariejan Aug 18 '14 at 14:54
  • either with channels or a mutex (see if the edits help). – JimB Aug 18 '14 at 15:04
  • Just as an aside - I much prefer the channel fanning as described in this video: https://www.youtube.com/watch?v=QDDwwePbDtw to relying on other synchronization mechanisms such as Mutexes. They are easier to reason about if you're coming from another language sure (it was the same for me coming from C#) - but the techniques described in that video make much more sense in the Go ecosystem. Just leaving it here as an alternative. – Simon Whitehead Aug 18 '14 at 22:37
  • As it turns out, it's much better for me to have Acia6551 (or Unit) implement io.Reader and io.Writer. Thanks for pointing that out and telling me about `-race`, which was actually quite useful. – Ariejan Aug 19 '14 at 07:33
  • 3
    Adding to what JimB wrote, an even better idea is to use *unbuffered* channels during development. If you don't get deadlocks with unbuffered channels, you still won't if you add buffering later (adding buffering becomes merely an optimisation step). This strategy is a good way to clearly think through the concurrency in the design and in particular getting rid of cyclic dependencies that often lead to deadlock. – Rick-777 Aug 19 '14 at 15:30