Netdev

[scottfeldman]

Hello!

I converted TinyGo's rtl8720dn driver to netdev and it's working great. All the tests are passing. Here are the changes I made to rpc.go. Please review and provide feedback. Thank you.

The driver code is here. rtl8720dn.go changed the most.

https://github.com/scottfeldman/tinygo-drivers/tree/netdev/rtl8720dn

List of changes in this PR:

1) Remove the err return from all of the functions since err is always nil. This simplifies the error handling logic; the hw error code is in the "result" value, so we don't need err.

2) Use all lower case for function name and lower-case *RTL8720DN structure so funcs and structure aren't exported. This is mostly because of 3) below.

3) Remove the semaphore. Locking in the driver moves up to the netdev level.

4) Remove this code. It doesn't do anything and is repeated many time.

    var result int32
    x := binary.LittleEndian.Uint32(payload[widx:])
-   if x >= 0x80000000 {
-       result = int32(int(x) * -1)
-   } else {
-       result = int32(int(x))
-   }
    result = int32(x)

5) Don't generate *[]byte out parameters, but rather use the more simple []byte. That's more Go style. The differences looks like:

    r.read()
    widx := 8
    // value : out []byte
    value_length := binary.LittleEndian.Uint32(payload[widx:])
    widx += 4
    if value_length > 0 {
-       copy(*value, payload[widx:widx+int(value_length)])
+       copy(value, payload[widx:widx+int(value_length)])
        widx += int(value_length)
    }
-   *value = (*value)[:value_length]

[sago35]

Thank you very much. Probably a good change.

I will now check the combination with netdev.

[sago35]

Which of the following is correct?

$ diff ../../../github.com/sago35/go-erpcgen/rtl8720dn/rpc.go ../../../tinygo.org/x/drivers/rtl8720dn/rpc.go
3019c3019
< func (r *rtl8720dn) rpc_wifi_get_mac_address(mac *uint8) int32 {
---
> func (r *rtl8720dn) rpc_wifi_get_mac_address(mac []uint8) int32 {
3029,3031c3029,3031
<       // mac : out uint8
<       *mac = payload[widx]
<       widx += 1
---
>       // mac : out []uint8
>       copy(mac, payload[widx:widx+18])
>       widx += 18

[scottfeldman]

Oh, yes! Thanks for catching that one; I wanted to talk to you about it.

The tinygo.org one is correct. By correct, I mean it was manually modified for this one function get_mac_address. The original function generated by go-erpcgen is wrong as it only copies one byte into *mac.

I looked at the lexer/parser code to see if I could make it generate the correct code for rpc_wifi_get_mac_address(out uint8[18] mac). I couldn't figure it out. Could you fix it? I like the generated function being Go-like and taking a []uint8 and doing the copy(...) for 18 bytes.

[sago35]

I looked at the lexer/parser code to see if I could make it generate the correct code for rpc_wifi_get_mac_address(out uint8[18] mac). I couldn't figure it out. Could you fix it? I like the generated function being Go-like and taking a []uint8 and doing the copy(...) for 18 bytes.

@scottfeldman Now that I was able to make the change, I will create and update the PR after merging this PR.

[scottfeldman]

Thank you!