Update configuration APIs

akif999 commented 1 year ago

https://github.com/akif999/drivers/issues/2#issuecomment-1414572421

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

Config 型に Stringer を実装する

Config 型に対して paramsToBytes を実装する

主な変更点
- config.go を追加
  - 新たに Config 型を定義
  - paramsToBytes を config.go へ移動
  - bytesToParams を追加
  - type Config に String() を実装
- e220.go
  - ReadRegister()
    - read 時の先頭 3byte (cmd + addr + length) を除いて []byte を返すように変更
      - 先頭の情報は API 使用者は既に持っている情報であり不要なため
  - writeRegister()
    - 処理完了後に 50ms wait するように変更
      - 8a6cb9cfc27b2ef31059f2353c55bc5c803d3ba1 での変更を一部戻した
      - レジスタアクセスの後、すぐに再度レジスタアクセスをすると失敗するため待つ必要があるため
  - WriteConfig() / ReadConfig()
    - []byte でなく Config 型を扱うように変更
- examples/e220/conf/main.go
  - 変更した WriteConfig() / ReadConfig に合わせて更新

sago35 commented 1 year ago

func (d *Device) WriteConfig(cfg Config) error で実施している各種エラーチェックは、 type Config 側で実施した方がよさそうだがうまく言語化できない。

akif999 commented 1 year ago

https://github.com/akif999/drivers/pull/6#issuecomment-1427262331 Config のバリデーションをどこでどうするべきかについて検討した結果。

以下より、案 3 を採用する。

func (d *Device) WriteConfig(cfg Config) error で実施している各種エラーチェックは、 type Config 側で実施した方がよさそうだがうまく言語化できない。

validation は Config に持たせたほうが良い気もするんだけど、そうじゃない気もする。 byteTo とか toByte とかやってる分には Validation は要らんのでは？という気持ちはある。

RssiAmbient とか、パラメータ毎に個別に設定するときは validation してもいいと思うけど。でも、メンバーに直接アクセスできる現状だとその手のチェックはできないし、わざわざメソッド化してまでやりというと。。。

現状
- WriteConfig() にて引数で受け取った Config 型の各値をチェックしている
変更案
1. validation 処理自体を削除する
  - メリット
    - 変更規模が小さい
    - 設計が簡単
  - デメリット
    - 不正なパラメーターが与えられた場合に、無視して有効なビットのみを使用する動作となる
      - ユーザーにパラメーターが不正であることを知らせられなくなる
2. Config 型のメンバーそれぞれへの Set メソッドを作成してそれらで validation を実施する
  - メリット
    - validation を実施してユーザーにパラメーターが不正であることを伝えられる(変更前と同じ)
  - デメリット
    - 他の実装の体系に比べて大きな実装になる(?)
3. Config 型に validation を実施するメソッドを持たせる
  - メリット
    - 変更規模が小さい
    - 設計が簡単
    - validation を実施してユーザーにパラメーターが不正であることを伝えられる(変更前と同じ)
  - デメリット
    - 変更前とさほど設計に変わりがない(改善度合いが少ない)

akif999 commented 1 year ago

https://github.com/akif999/drivers/pull/6#issuecomment-1428913615

Config のバリデーションをどこでどうするべきかについて検討した結果。以下より、案 3 を採用する。

案 3 に基づき以下にて変更した。 7e3740a86b469889eeb5720c0f6b183979dd8c1d c998f6aee4d034e66c8abbd723a7e6cfb6fd3cdd

備考
- Config.Validation() は export とした
  - ユーザーが任意のタイミングで作成した E220 の設定値を validation したい場合もあると考えたため
- ‘Config.errors‘ の各値に対応する状態 (やや煩雑で良くない設計だと考えている)
  - nil → Config.Validate() が一度もコールされていない
  - 0 → Config.Validate() をコールした結果、不正なパラメータはなかった
  - !0 → Config.Validate() をコールした結果、不正なパラメータはあった

akif999 / drivers

Update configuration APIs #6