[E220] Improve driver - Githubissues

akif999 commented 1 year ago

Improve E220 driver.

akif999 commented 1 year ago

examples/e220/conf の例は、 ReadConfig() 使って config 読み出しにすると良さそう

コンフィグ用の example を作成しました。工場出荷状態のコンフィグを書き込んでから読み出すような手順にしました。

M0 / M1 をつなぐつもりなら、 examples/e220/tx などでも config 読み出しをしておくのがよさそう

tx の example に config 読み出しの手順を追加しました。

io.Reader と io.Writer になるように作るとよりよい

ただし machine.UART は ↓ の問題があるのでうまく作る必要がある

https://github.com/tinygo-org/tinygo/pull/2739

https://github.com/akif999/drivers/issues/2#issuecomment-1414572421 にて対応する。

ReadRegister() 内のループは明らかに良くないので修正したが、無限ループ入るのでこれはこれで良くない

要検討

手前の 100ms wait は要らない

一旦 5sec 以上処理が継続している場合はタイムアウトとするようにしました。 100ms の wait は削除しました。

akif999 commented 1 year ago

その他の主な変更点。

IsReceived() としていた API は uart.Buffered() をそのまま wrap した API とし、名前を Buffered へ変更
- 設計初期は IsReceived と IsBuffered の 2 つの役割が重複した API を用意していたが 1 つにまとめることとしたため
WriteRegister / ReadRegister の内部処理を unexport な関数へ共通化
- write length のみ分岐が必要であるが、それ以外は共通化可能であるため
memory allocation をしないように変更
- 各所で slice を作成し、 append により reallocation していく処理になっていたため
SetMode() 後の待ち時間を 10ms -> 30ms へ変更
- MCU 起動後すぐに SetMode すると、E220 の AUX がまだ立ち上がっていない間に SetMode が完了して E220 へのアクセスを実施して失敗してしまう (既知)
- E220 起動後 AUX は 20ms 程度 Low level となるため、10ms の wait でもまだ早すぎる場合があったため、30ms へと変更した
Write(Read)Register の処理後、50ms wait するように変更
Write(Read)Register の処理後、再度 write/read いずれかの処理を実行する際、早すぎると E220 から invalid format error (= 0xFF 0xFF 0xFF) が返送されるため
- 30ms -> NG, 40ms -> NG 50ms -> OK であったため 50ms を設定
- 本当は msg の write 処理を含めて aux が High になるのを待ってから処理するようにした方がよいかもしれないので要検討

akif999 commented 1 year ago

2023/01/30

io.Reader 関連

io.Reader は bufio.Reader もしくは bufio.Scanner 辺りが動く形になるといいなぁ、と思います。使うのが本当に楽 (とい> うか Golang らしく書ける) になるので。

https://github.com/akif999/drivers/issues/2#issuecomment-1414572421 にて対応する。

あと、今の時点だと []byte{} ですらなくて byte x N なものが返ってるので、どこまでがひと塊か、というのも分からない> 感じです。そのあたり分かったほうが良いです。例えば、 RSSI とかを追加できるモードがあったと思うけど、その時は、以下のような形で返せるべきだと思う。で、そのあたり無視して丸っと data だけ欲しい人は、 io.Reader を使う、、、みたいな。type Packet struct { RSSI uint8 Data []byte}

Driver のレイヤでの Packet 単位化には対応しない。別途上位のレイヤとして https://github.com/akif999/drivers/issues/2#issuecomment-1414572421 にて対応する。

io.Reader 化は少しあとでもよいと思う。たぶん。パケットの単位で Callback もしくは Polling できるとよいと思う。※今> は、 Buffered() かどうかで 1 byte 単位で受け取ってるけど、パケットの単位で取れてほしい

Driver のレイヤでの Packet 単位化には対応しない。別途上位のレイヤとして https://github.com/akif999/drivers/issues/2#issuecomment-1414572421 にて対応する。

その他

あと、 go blink() すると、 goroutine が動いている、ぐらいの確認にしかなってなくて、 wdt 的な目で見て動いているかど> うかを判断する、という用途に使えないので注意が必要。多分、 for ループの中で回す、ぐらいのほうが良いと思う。実質> 的には「常に点滅している」だけになってるので。

https://github.com/akif999/drivers/issues/2#issuecomment-1414572421 にて対応する。

device.ReadConfig() が入ってるのは良いね。 examples/e220/rx 側にも入れておくと良さそう。

https://github.com/akif999/drivers/issues/2#issuecomment-1414572421 にて対応する。

ReadConfig() の戻り値は ([]byte, error) じゃなくて、 type Config []byte にしといて Stringer 作っておくと良さそう。で、> それを使って paramsToBytes() とかを作れそう。多くの場合、cfg, _ := > device.ReadConfig()cfg.Channel(8)device.WriteConfig(cfg)というような形で設定したいと思うはず。

https://github.com/akif999/drivers/issues/2#issuecomment-1414572421 にて対応する。

後は、 data, _ := device.ReadByte() で取り出した値には 0xFF とかが入るわけなのですが、それが含まれずに payload だけになったものを返す IF が欲しい気がする。この辺り、なんだっけ？独自方式かどうか、とかに依存してる気がするけど、どうしたものか。少なくとも hello って送って届くのが []byte{0xFF, 0xFF, 'h', 'e', 'l', 'l', 'o'} なのはなかなか難しいとい> うか。fmt.Fprintf(device, "hello\n") ってしたのを受け取ると "hello\n" になって欲しい。そのためには、パケットの概念が必要になってくる。

Driver のレイヤでの Packet 単位化には対応しない。別途上位のレイヤとして https://github.com/akif999/drivers/issues/2#issuecomment-1414572421 にて対応する。

akif999 commented 1 year ago

io.Reader/Writer 化以外の TODO をまとめる。

下記は https://github.com/akif999/drivers/issues/2#issuecomment-1414572421 としてまとめ直しているため、そちらを参照のこと。

TODO

() 内の項番は優先度順。

(1) 受信をパケット単位で実施できるようにする
- Callback or Polling で実装する
- パケット型のイメージ type Packet struct { RSSI uint8 Data []byte }
  - RSSI byte が付与されるかどうかは E220 のコンフィグによる
(2) パケットを取り出す際に、トランスペアレントモードか否(固定送信モード)かをもとに、payload のみを取り出す I/F を追加する
- 送信側も現状が先頭 3byte を addr + ch として扱う固定送信モードを前提とした送信 I/F となっているので改善する
(3) ReadConfig の戻り値を type Config []byte とする
- Config 型に Stringer を実装する
- Config 型に対して paramsToBytes を実装する
(4) examples 関連
- (重要度低につき後でまとめる)

課題

(1)
- UART のバッファからパケットを取り出す際に、何を元に 1 パケットと判断するか
  - パケットのデータにはその始まりと終わりを示すデータは付与されていない
  - 現状の実装案は以下
    - rxTask を goroutine で回しておいて、UART がバッファリングされたらまとめて取り出して、ユーザへ Packet として渡しつつコールバックする
(2)
- ....

sago35 commented 1 year ago

(1) 受信をパケット単位で実施できるようにする

多分、自分がミスリードをしている感じがしてきました。現状の e220 firmware だとパケット単位でとるのは難しい (というか、実質無理) と思います。なので、パケット型のイメージで取得するのは無理かな、という感じですね。

ではどうするか、というと、 (1) の話が不要となり、単に io.Reader を実装するように作るのが正しくなってきます。で、その io.Reader から読めるバイト列に length 等の要件を (この driver とは別、あるいは同じでも 1 段高いレイヤーに) 追加する必要があります。

(2) パケットを取り出す際に、トランスペアレントモードか否(固定送信モード)かをもとに、payload のみを取り出す I/F を追加する

これは、 ↑ の対応 (length 等も送る) をしたうえでサブパケット長以下のデータのみ送るようにしないと成り立たないような。なので、いったんはトランスペアレントモードのみを題材にするとシンプルに実装が進みそう。

(3) ReadConfig の戻り値を type Config []byte とする

これはそのまま進めると良いと思う。

akif999 commented 1 year ago

↑ に関連。

あと、できれば単なる driver はなるべく goroutine は使わないほうが良いです。今回の場合だと、 io.Reader にすることにしたので、以下のように読み込んでおけばよいです。で、ブロックされるのをそもそも嫌がる人は、 goroutine 経由で > Read するし、サイズ決めつつ読みたい場合は p のサイズで調整できるので。多分これでいいと思う。細切れになりすぎるのを嫌がる場合は、 bufio.Reader などが使える。 https://github.com/akif999/drivers/blob/cb5d19db5044909baf2fd8e61b115c92bb6c951e/rtl8720dn/rtl8720dn.go#L59-L69 write 側も同じような感じで、基本的にはブロックさせつつ UART 送信していけばよい。 block を嫌がる場合は bufio.Writer が使える。

akif999 commented 1 year ago

https://github.com/akif999/drivers/issues/2#issuecomment-1412919075 上記からアップデートした TODO をまとめる。

(1) io.Reader, io.Writer に対応する (DONE)
- Reader は drivers の他の実装(以下)と同様に実装する
  - https://github.com/tinygo-org/tinygo/blob/v0.26.0/src/os/file_other.go#L41-L61
- examples に io.Reader / io.Writer を用いた送受信の例を実装する
(2) 送信 I/F が先頭 3byte を addr + ch として扱う固定送信モードを前提とした送信 I/F となっているので改善する (DONE)
- トランスペアレントモードか否かで先頭に addr + ch を設定するかを分岐するようにする
(3) ReadConfig の戻り値を type Config []byte とする (DONE)
- Config 型に Stringer を実装する
- Config 型に対して paramsToBytes を実装する
(4) examples 関連 (DONE)
- blink の入れ方を変える
- rx の example に ReadConfig を入れる
- (examples に関係ないが) buildtag を足す (CI 対策)
- passthrough の example を足す (下記参照)
(5) Reader から読める byte 列に length 等の情報を含めたものを上位のプロトコルとして追加する (ここでは対応しない)
- 今の driver (io.Reader / io.Writer) の一段上のレイヤーに実装する
- ISO15765-2 として載せるという選択肢もあり
- 意図としては、Tx/Rx 側同士で Packet (Message) という単位での通信ができるようにしたい (要確認)

(1, 2) 関連

io.Writer 側は、 addr / ch はどこかで持っておく必要がある。基本的には *Device 内でデフォルト送信先として保持する感じかな。そのうえで、 Write() がコールされたら、そこに単純に送り付けると良い。

Write に addr と ch を持たせる、のではなく、 Write() コール時の送信先の addr と ch を Device が持つ、が正しいかな。なので、関数名は適当ですが func (d Device) SetDefaultAddress(addr uint16, ch uint8) 的なものがあると良いかな。

(5) 関連

上位のプロトコルを決めてしまいつつ、サブパケットに必ず収まる形で送信することにすれば、上記にとらわれずに正しいサブパケットの切れ目は認識できそう。ただ、受信漏れとかがもし発生する場合は、それを元に戻すための仕組みが> 必要。

送信を必ず固定長にする、とかも一つですな。デコードらくちん。例えば、 2byte 捨てて、残り 64byte を読む、とかに> すると簡単。driver でどこまでやるべき？みたいなのはあるんだけど、元の firmware がいまいちなので、高砂としてはこれぐらいの制限をしてしまってもよい気もする。

上記で固定長にする、っていうのを実装する場合は、 Write() の実装内で、例えばあるサイズより短いなら length とかを> 足して送信 (ISO15765-2 の SF のイメージ)、それ以上なら MF (FF / CF) のイメージで分けつつ送信、という感じ。受信側> は、 SF にせよ MF にせよ固定長読んでから length とかを判定するイメージ。固定長で送られてくるので、先頭 2byte 捨> てたりはやりやすいはず。

(4) 関連

以下の example を足す

package mainimport (
    "io"    "machine"    "os"    "time"    "tinygo.org/x/drivers/e220")
var (
    uart = machine.DefaultUART    m0   = machine.D4    m1   = machine.D5    tx   = machine.UART_TX_PIN    rx   = machine.UART_RX_PIN    aux  = machine.D6    led  = machine.LED)
func main() {
    device := e220.New(uart, m0, m1, tx, rx, aux, 9600)
    err := device.Configure(e220.Mode0)
    if err != nil {
        fail(err.Error())
    }
    go func() {
        io.Copy(os.Stdout, device)
    }()
    w := io.MultiWriter(os.Stdout, device)
    io.Copy(w, os.Stdin)
}
func fail(msg string) {
    for {
        println(msg)
        time.Sleep(1 * time.Second)
    }
}

sago35 commented 1 year ago

別途 Reader から読める byte 列に length 等の情報を追加する今の driver の一段上のレイヤーに実装する

これは、後で良い (io.Reader 対応とは別が良い) です。雰囲気的には、 DiagCAN / ISO15765-2 のような層をかぶせるとよいと思う。そして、 Driver 自体にはいらないのかもしれない。

io.Reader / io.Writer の上のレイヤーとして ISO15765-2 を載せる、ってのは結構面白いかもしれない。 (ただし CAN の 8byte ではなく今回だと 200 byte ? までのサイズにするとか)

そして割と役立つのかもしれない。 (もちろん、もっと効率の良いやり方はいっぱいあると思う)

akif999 commented 1 year ago

↑ のコメント(下記 ID )の内容を↑↑のまとめに反映済み。 https://github.com/akif999/drivers/issues/2#issuecomment-1414581060

akif999 commented 1 year ago

実装しながら気になった点。

WriteMsg
- 今は AUX Pin の状態を確認して、E220 が送信可能な状態となったら送信している
  - 正しい実装と考えているが、ユーザーが AUX 未接続でルーズに動作テストをしたい場合に動かないコードとなっている
  - AUX を High level 固定として使用してもらえば動くのは動く

sago35 commented 1 year ago

AUX を High level 固定として使用してもらえば動くのは動く

こういうケースは machine.NoPin を使ってもらう、ってのが一つですな。

akif999 / drivers

[E220] Improve driver #2

io.Reader 関連

その他

TODO

課題