SDUpdaterに対応する

ronron-gh commented 3 months ago

NoRiさんのｽﾀｯｸﾁｬﾝ向けSDUpdaterに対応させる。

現状NoRiさんのSDUpdaterはCore2向けのため、他の機種向けにビルドするときはコンパイルオプションでSDUpdater用コードを無効化できるようにする。
他のソフトとの設定ファイルの共通化を検討する。YAML、タカオさんのライブラリを使用。

mongonta0716 commented 3 months ago

@NoRi-230401 で呼べないですかね？

NoRi-230401 commented 3 months ago

今、メール案内から承諾してここにたどりつきました。

ronron-gh commented 3 months ago

@mongonta0716 @NoRi-230401 コメントくださったおかげで、@で候補に出てくるようになりました。

本題ですが、こちらのコミットで、まずはSDUpdater対応してみました。 a5d5013a4393f854bf8451d024175b7a16572826

platformio.iniの[env:m5stack-core2-sdu]でビルドしたときだけSDUpdater対応になるようにしてみました。もっと良い方法がありましたらご遠慮なくコメントください。サーボPIN設定はひとまずNoRiさんのservo.txt方式を取り入れましたが、これからYAML方式を検討していきたいと思います。

NoRi-230401 commented 3 months ago

さっそく、ビルドして使用してました。私の「動けばいい的ソフト」から汎用性を持たせることができるよう変更するには、こうゆう風にすればいいんだと感心しております。改めて取り込まれた自分のソースを見るとエラー処理がいい加減で申し訳ないです。

mongonta0716 commented 3 months ago

SDUpdater.hとSDUpdater.cppの関数をstackchan-arduinoライブラリで持ってもいいかもしれませんね。

NoRi-230401 commented 3 months ago

提案です。 "PROG_NAME"（lobbyで表示されるソフトの名前）を定義している場所は、

SDUpdater.cppで２３行目　　const String PROG_NAME = "AiStackChan2FuncCall";

(１)．これをmain.cppで定義するようすると、他のソフト毎にSDUpdater.cppの変更する必要がない。 (２)．この名前自体を設定ファイルから読取方式にするとmain.cppもソフト毎に変更する必要がないのでいいかも。

すいません、追記です。よく考えたら(２)は、SDU_lobby（）は、M5.begin直後に呼ばれるので無理そう。結局(１)のmain.cppに埋め込むのがいいのでは。

NoRi-230401 commented 3 months ago

今回のSDUpdaterとは関係ないのですが、前に調べていてわかったのですが、「Error attaching servo xError attaching servo y」のエラーが出ていています。（シリアルモニタ） main.cpp 225-230 の合否判定が逆になっているようです。この機会に訂正お願いします。

ronron-gh commented 3 months ago

ご確認ありがとうございましたm(__)m

SDUpdater.hとSDUpdater.cppの関数をstackchan-arduinoライブラリで持ってもいいかもしれませんね。

確かにそうですね。十分に汎用的になったところで、stackchan-arduinoライブラリへ移動したいです。

"PROG_NAME"（lobbyで表示されるソフトの名前）を定義している場所

確かに、SDUpdater.cpp内で定義すると他のソフトで使いづらいですね。提案ですが、このようにmain.cppでSDU_lobby()を呼ぶときに引数で渡すのはいかがでしょうか（こちらで改造し、問題なく動作しました）。

  SDU_lobby("AiStackChan2FuncCall");

「Error attaching servo xError attaching servo y」のエラーが出ていています。（シリアルモニタ）

情報ありがとうございます。修正を取り込みます。

NoRi-230401 commented 3 months ago

SDUpdater.cppの修正いいと思います。よろしくお願いします。

ronron-gh commented 3 months ago

上記の修正をコミット( ed101aee917ba90f4bed2d7b96246b257085612b )したので、YAMLのほうに着手しようと思います。細かな方針について次のようなことを考えています。ご意見ありましたらお願いいたします。

YAMLファイルがSDカードに無い場合は、レガシーなTXTファイルを読み込む。TXTファイルから読み込んだ場合はYAMLファイルに書き起こしてSDカードに新規保存することで、ユーザーのYAML移行を促したい。
SC_ExConfig.yaml（アプリ毎の設定）は、SDUpdaterで複数アプリが共存する場合はアプリと同じ数のSC_ExConfig.yamlが出来るのか、一つのSC_ExConfig.yamlに複数アプリの設定を保存するのかを決めたい。以下の理由で自分的にはアプリ毎のフォルダを作った方が良い気がしていますが、他に妙案等あればお願いいたしますm(__)m
- アプリ毎にファイルが出来る場合は、アプリ毎にファイル名を変えるか、アプリ毎のフォルダを作って保存する必要がある。（そもそもYAMLファイルに限らずSDカード直下にいろいろなアプリが作ったファイルが混在すると使い勝手が悪いので、アプリ毎のフォルダはあった方がよいのではないか）
- 一つのファイルにすればスッキリするが、あるアプリが間違ってファイルを壊すと他のアプリにも影響する。

mongonta0716 commented 3 months ago

アプリ側で決められるように、SC_ExConfig.yamlという名前を変えられるようにしましょう。SC_BasicConfig.yamlとSC_SecConfig.xmlは名前を固定してもいいような気もしますね。

StackchanSystemConfigのコンストラクタでSC_BasicConfig.yamlではなく、SC_ExConfig.yamlの名前を指定するようにしましょうか。

mongonta0716 commented 3 months ago

変更しました。

https://github.com/mongonta0716/stackchan-arduino/commit/84560891022382dd4808fd2c92371ca6b70d4dfb

NoRi-230401 commented 3 months ago

・私も二人の意見とほぼ同じなのですが、縛りをきつくすると窮屈さを感じるのでSC_BasicConfig.yamlとSC_SecConfig.xmlとライブラリを使えば、そのまま便利に使える共通部品として便利になる。それ以外は、設定ファイルも各アプリで名前を変えて自由に使えるようになるといいと思います。

・レガシーTXTをサポートは嬉しいです。ただ、心配なのが、SD書込みは失敗がありそうなので、その時にファイルが破損しないかが気になりました。自分の経験だと、PCを使用して問題ないSDへの書込みが、M5StackCore2直で書込みが失敗しファイルが破損することがあるので（SDの相性？？）書込みや止めた方が無難と思っています。読み込みは比較的大丈夫だと思います。もしかして、私の固有の問題かもしれないのです。

訂正です。よく考えて見たら、もともと存在なかったファイル作成するんだから失敗して大丈夫ですね。すいません、よけいな懸念ですね。

・アプリ毎のフォルダもあった方が、アプリが他の影響うけにくいので作りやすいですね。アプリ毎フォルダ下にアプリ専用の設定ファイルを持ってくるのがやはり自然ですね。共通部分だけの設定ファイルの位置と名前とを決めるのかなあ。

mongonta0716 commented 3 months ago

書き込みと相性は確かに怪しい部分があります。特にCoreS3、、、KIOXIAとSandisk以外は信用できませんね。。。あまり長いフルパスファイル名も使えないので、、、下記のようにしてしまうとか、、、 /yaml/SC/Basic.yaml（読み込み専用） /yaml/SC/Secret.yaml /yaml/<フォルダも含めてAPPで自由>.yaml

フォルダも自由ぐらいで収めておいてもいいかも。

ronron-gh commented 3 months ago

ご意見ありがとうございました。書き込みのトラブルは確かに心配ですね。新規作成のみとはいえ、内容が中途半端なファイルが出来て次回起動時に読み込みエラーという可能性もあるので、ひとまず書き込みは無しで進めたいと思います（単に、YAMLが無ければレガシーTXTを読み込む、に留める）。

ルールを決め過ぎてアプリ開発に参入しづらくなるのも確かに避けたいですね。「stackchan-arduinoライブラリを使えば簡単にベース部分を作れる」というコンセプトも大事にしたいですね。それを踏まえると、以下のような説明のしかたが良いでしょうか。

stackchan-arduinoライブラリを使えば以下の設定ファイルから簡単に共通的な設定を読み込める。（あくまで推奨）
- /yaml/SC_Basic.yaml ・・・共通的な設定
- /yaml/SC_Secret.yaml ・・・共通的な設定（個人情報）
アプリ固有の設定ファイルや一時ファイルは以下のアプリ固有フォルダを作って保存することを推奨する。
- /app/(アプリ名)

@mongonta0716 アプリ固有のファイルはyamlに限らないと思ったため、フォルダ名を"app"にしてみました。

ronron-gh commented 3 months ago

@mongonta0716 stackchan-arduinoライブラリに関する提案です。YAMLがない場合にレガシーTXTを読み込むことを考えていますが、loadConfig()およびloadSecretConfig()でYAMLを開けたかどうかの判定がされているので、開けなかった場合に以下のようにコールバック関数を呼ぶようにしてはどうでしょうか？コールバック関数は継承クラスにてオーバライド可能とし、どのように実装するかはアプリ開発者の自由としたく思います。（もし問題なければ、こちらでデバッグしてFork、PRさせていただきます。）

void StackchanSystemConfig::loadConfig(fs::FS& fs, const char *app_yaml_filename, uint32_t app_yaml_filesize, const char* basic_yaml_filename, uint32_t basic_yaml_filesize) {
    M5_LOGI("----- StackchanSystemConfig::loadConfig:%s\n", basic_yaml_filename);
    M5_LOGI("----- app_yaml_filename:%s\n", app_yaml_filename);
    fs::File file = fs.open(basic_yaml_filename);
    DynamicJsonDocument doc(basic_yaml_filesize);
    if (file) {
        DeserializationError err = deserializeYml(doc, file);
        if (err) {
            M5_LOGI("yaml file read error: %s\n", basic_yaml_filename);
            M5_LOGI("error%s\n", err.c_str());
        }
        serializeJsonPretty(doc, Serial);
        setSystemConfig(doc);
    } else {
        Serial.println("ConfigFile Not Found. Default Parameters used.");
        // YAMLファイルが見つからない場合はデフォルト値を利用します。
        setDefaultParameters();

        basicYamlNotfoundCallback();    //継承クラスでオーバライド可能とし、レガシーTXT読み込みなどを実装したい
    }
    if (_secret_config_filesize > 0) {
        loadSecretConfig(fs, _secret_config_filename.c_str(), _secret_config_filesize);
    }
    if (_extend_config_filesize > 0) {
        loadExtendConfig(fs, app_yaml_filename, app_yaml_filesize);
    }
    printAllParameters();
}

void StackchanSystemConfig::loadSecretConfig(fs::FS& fs, const char* yaml_filename, uint32_t yaml_size) {
    M5_LOGI("----- StackchanSecretConfig::loadConfig:%s\n", yaml_filename);
    File file = fs.open(yaml_filename);
    if (file) {
        DynamicJsonDocument doc(yaml_size);
        auto err = deserializeYml( doc, file);
        if (err) {
            M5_LOGE("yaml file read error: %s\n", yaml_filename);
            M5_LOGE("error%s\n", err.c_str());
        }
        if (_secret_config_show) {
            // 個人的な情報をログに表示する。
            M5_LOGI("=======================================================================================");
            M5_LOGI("下記の情報は公開してはいけません。(The following information must not be disclosed.)");
            M5_LOGI("");
            serializeJsonPretty(doc, Serial);
            setSecretConfig(doc);
            M5_LOGI("");
            printSecretParameters();
            M5_LOGI("ここまでの情報は公開してはいけません。(No information should be disclosed so far.)");
            M5_LOGI("=======================================================================================");
        }
        else{
            secretYamlNotfoundCallback();    //継承クラスでオーバライド可能とし、レガシーTXT読み込みなどを実装したい
        }
    }
}

NoRi-230401 commented 3 months ago

上記の「stackchan-arduinoライブラリに関する提案」、素晴らしいと思います。

mongonta0716 commented 3 months ago

良いですね😊 よろしくお願いします🙇

ronron-gh commented 3 months ago

ありがとうございます！方針がまとまってきたので実装してみます。

ronron-gh commented 3 months ago

@mongonta0716 deserializeYml()の変換結果ですが、" ”で囲った文字列が数字のみだと、JSON変換後が数値(float)になってしまいます。docをserializeJsonPretty()で表示した結果が以下のようになっています。YAMLDuinoの問題ですかね🤔 強制的に文字列として認識させられればいいのですが（" "で囲えばいいのかと思ったのですが）

JSON変換結果 (serializeJsonPrettyで表示)

  "wifi": {
    "ssid": "1234ABCDEF-2G",        <- アルファベットが含まれていれば問題なし
    "password": 1.2345657e12       <- YAML上は"1234567899999"という文字列だが、JSON変換したら数値(float)になった
  },

ronron-gh commented 2 months ago

とりあえずYAMLDuinoのissueで報告しておきました。

ronron-gh commented 2 months ago

もう対応してもらえました☺ ver.1.4.1でリリースされています。ただしArduinoJsonは7.xに上げないといけないようです。

https://github.com/tobozo/YAMLDuino/issues/20#issue-2355479202

tobozo commented 2 months ago

hi everyone 👋 sorry about breaking stackchan :bow:

YAMLDuino 1.4.2 is now available with support for both ArduinoJson 6.x and 7.x so you can migrate smoothly

it is already available in platformio library registry and will propagate soon to arduino library registry

ronron-gh commented 2 months ago

Thank you for supporting ArduinoJSON 6.x. It seems that ArduinoJSON 7.x will not affect Stackchan, but since the popular software AI Stackchan2 uses ArduinoJSON 6.x, I think it will be easier to introduce YAMLDuino. 😊

mongonta0716 commented 2 months ago

すいません。気づくの遅くなりました。

YAMLDuinoの1.4.2とArduinoJSON 7.xですね。。。( ..)φ

ronron-gh commented 2 months ago

あ、1.4.1で一旦ArduinoJSONは7.x限定になりましたが、先ほど1.4.2でArduinoJSON 6.xと7.xの両方に対応してくださったようです。

ronron-gh commented 2 months ago

お待たせしておりましたが、こちら 55ba12e187fca8cd8ab1bd719fe7dbe595d11988 のコミットで、YAMLによる設定に対応してみました。タカオさんのライブラリに対する修正はタカオさんのリポジトリにPRしました。https://github.com/mongonta0716/stackchan-arduino/pull/4 ご意見などありましたらご遠慮なくお知らせくださいm(__)m

主な内容：

SDUpdater.cpp と SDUpdater.hをタカオさんのライブラリに移動
YAMLファイルが無い場合はレガシーなTXTファイルを読みに行く
レガシーなTXTファイルすら無い場合は、サーボPINのみデフォルト値を設定
アプリ固有設定（SC_ExConfig.yaml）はこの後対応します（これは「アプリ毎に自由にやってください」なので）

@NoRi-230401 SDUpdater.cpp をタカオさんのライブラリに移動する際、servo.txtの読み取り関数はSDUpdaterに依存するものではないと思い、誠に勝手だと思いつつ削除してしまいました。その代わり、今回の対応でYAMLが無ければservo.txtからサーボピン番号を読み取るようにしましたので、ご活用いただけますと幸いですm(__)m

NoRi-230401 commented 2 months ago

@ronron-gh さん、対応ご苦労さまでした。 servo.txt も素晴らしい対応になったと思います。ありがとうございます。コンパイル時に、複数の機種に対応する方法もとっても参考になりました。

私の方は、変更内容をもとにして私が作成したソフトから修正していきたいと思います。昨日ソフト（SDU-WebDav）を出したので、まず、それに変更をかけてみようかと考えています。

ronron-gh commented 2 months ago

ありがとうございます。大変恐縮ですm(__)m タカオさんのリポジトリにマージされるまでは、platformio.iniでは次のように私がForkしたリポジトリを指定いただければ使えると思います。

lib_deps = 
    https://github.com/ronron-gh/stackchan-arduino.git#support_sdupdater

宜しくお願いいたします。

ronron-gh commented 2 months ago

@NoRi-230401 マージいただけたので、ライブラリは以下でお願いいたします。

lib_deps = 
    https://github.com/mongonta0716/stackchan-arduino.git

ところで、Core2 V1.1だとmenu.bin（元はM5Core2-Launcher-2.0.13.binですね）をロード後に画面が真っ暗のまま起動しない現象をこちらで確認しました。追々issueを上げるなどしたいと思いますが、取り急ぎご連絡です。

mongonta0716 commented 2 months ago

プルリクエストありがとうございました。m( )m 今はちょっと忙しいので、お誕生日会の後にライブラリ公開する予定でいます。

NoRi-230401 commented 2 months ago

Core2 V1.1 の menu.bin 不具合の情報ありがとうございます。確認できていませんでした。

ronron-gh / AI_StackChan2_FuncCall

SDUpdaterに対応する #2