OSレスでの組み込み製品開発における、キャラクタLCD画面状態遷移処理方法

Question

初めまして。

OSレスでの組み込みシステムについて質問させていただきます。

私は、「設定した時間に音を鳴らす」という単純な機能を持つ製品を開発しています。
設定変更用の入力ボタン（上下左右＋選択）と、16×2行のキャラクタLCDが備えられています。

下記製品のような操作部をイメージしてください。
LCD Keypad Shield For Arduino

ユーザーの入出力制御は、状態変数を用意し、状態に応じた処理を行っています。
基本的には、LCDの表示内容を変えるだけですが、
ユーザーが設定内容を変えた場合は、変更内容をEEPROMに保存する処理もあります。
なお、メインループの中でAD変換→冷却FAN制御→スピーカー制御→入出力制御のように順番に処理しています。

そこで、問題なのですが、

static uint16_t seq=0;

switch(seq) {
   case 0:
      lcd_puts("   MAIN MENU   ");
      if(sw == SW_UP) { seq=1; }
      break;

   case 1:
      lcd_puts("   MENU 1      ");
      if(sw == SW_DOWN) { seq=0; }
      if(sw == SW_UP) { seq=2; }
      if(sw == SW_SELECT) {
         eeprom_write("TEST");
         seq=0;
      }
      break;

このようなswitch文がcase500ぐらいまであります。
新しい機能を追加し、設定項目が増える度に、switch文が長くなり焦っています。

正直なところ、私は素人から毛を抜いた程度なので、このようなプログラムで良いのか判断できません。

OSレスの家電製品のプログラムを参考にしたいと思ったのですが、探しても見つかりませんでした。

なにか参考になるプログラムや参考書等ご存じでしたら、ご教示願います。
また、組み込みプログラムについてお詳しい方のアドバイスお待ちしております。

どうぞよろしくお願いします。

Answer 1

具体的に解決したい問題はなんですか? (teratailのヘルプを見ると、「具体的な問題を提示し、それに回答する」というのがここの基本スタンスのようなので)

OSレスの家電製品のプログラムを参考にしたいと思ったのですが、探しても見つかりませんでした。

まぁ、製品のプログラムを公開する、なんてことはまずあり得ないですね。

さて。
常識的に言って、その操作系で

このようなswitch文がcase500ぐらいまであります。

というのは明らかに設計がおかしいです。フラットに500の選択項目が並んでいる、というのは...ありえないとはいいませんが、ふつーの神経なら、それをグルーピングしてメニューを階層化します。全項目は500のままかも知れませんが、一時に気にしなければいけない範囲はそれでも随分限定出来るでしょう。

また、項目が少ないならそのプログラムでもいいかも知れませんが、500の項目それぞれに

      if(sw == SW_DOWN) { seq=0; }
      if(sw == SW_UP) { seq=2; }
      if(sw == SW_SELECT) {

なんていう処理を持たせるのですか? UPやDOWNが押されたときの遷移先、その項目の種類(設定値を変更する/なにかの機能を実行する)、変更する設定値、実行する機能(関数)へのポインタなんかを構造体にして、それをテーブル(配列)にして「メニューの情報」を分離して、実行部はそれを解釈するだけ、みたいな構成に出来れば随分管理はやりやすくなるのでは? 質問のプログラムに例を取るなら、

enum MENU { //メニューのIDを定義する
  //一部だけ値を指定するのはMISRA等では怒られるがサンプルということで
  NONE = -1,
  MAIN = 0,
  MENU1,
  MENU2,
  MENU_MAX
};
struct MenuItem {
  enum MENU id;       //メニュー項目のID
  char* Title;  //タイトル表示文字列
  enum MENU moveUp;   //UPキーの遷移先
  enum MENU moveDn;   //DOWNキーの遷移先
  char* eepDat; //SELECT時EEPROM書き込みデータ
  enum MENU moveSel;  //SELECTキーの遷移先
};
const struct MenuItem menuItems[] = {
  // id項目は不要とも言えるけど...あると意外と便利なことも
  // id,    Title,           ,  up,     down,   EEP,    sel
    {MAIN,  "   MAIN MENU   ",  MENU1,  NONE,   NULL,   NONE},
    {MENU1, "   MENU 1      ",  MENU2,  MAIN,   "TEST", MAIN},
    {MENU2, "   MENU 2      ",  MAIN,   MENU1,  "EXEC", MAIN},
};

static uint16_t seq = 0;
void doMenu(int sw) {
  int index = -1;
  int mv_tmp= NONE;
  char* eep_tmp = NULL;
  for (int i = 0; i < MENU_MAX; i++){
    if(menuItems[i].id==seq){
      index = i;
      break;
    }
  }
  if (index < 0) return;
  lcd_puts(menuItems[index].Title);
  switch(sw ){
    case SW_UP:
      mv_tmp = menuItem[index].moveUp;
      break;
    case SW_DOWN:
      mv_tmp = menuItem[index].moveDn;
      break;
    case SW_SELECT:
      eep_tmp = menuItem[index].eepDat;
      mv_tmp = menuItem[index].moveSel;
      break;
  }
  if(eep_tmp!=NULL){
    eeprom_write(eep_tmp);
  }
  if (mv_tmp >= 0) {
    seq = mv_tmp;
  }
}

なんて感じにまとめられればmenuItemsのデータの追加だけで要素は増やせるわけで。(条件によってはもっと簡素になるかも知れないけど、もっとメニュー動作の「例外」があって対応しなきゃいけなくなるのが現実か)

Comments

[thkana]

あと、「この状態のときはこのメニューはスキップ」なんてのをどう処理するか、なんてのがひとつのヤマかな。

[dakao]

具体的なコードまで示していただきありがとうございます。
検討してみます。

Answer 2

switch文でシーケンスを制御するのは、保守のことを考えるといつか破綻すると思います。
ステートマシン(状態遷移テーブル)で設計することをお勧めします。

参考になりそうなのは、こことか、
https://katono123.hatenablog.com/entry/20120325/1332650559

デザインパターンの一つである、Stateパターンで調べてみるといいかも。

Comments

[dakao]

Stateパターンについて勉強してみたいと思います。

Answer 3

・メニューに階層無し
・メニュー上下でのループなし
・SELECT後はMAIN MENUに戻る
と仮定して、こんな感じですかね？
メニュー表示処理とメニュー選択時の処理を分けておけば
選択時の処理関数と、メニュー内容の構造体への追加だけで済みます。

void menu1();
void menu2();
struct
{
    const char* menuText; // LCD表示
    void (*menuFunc)();   // SELECT時処理関数
} menu[] = {
    { "   MAIN MENU   ", NULL },
    { "   MENU 1      ", menu1 },
    { "   MENU 2      ", menu2 },
};

static uint16_t seq = 0;

void loop()
{    
    sw = ～;//sw取得

    lcd_puts(menu[seq].menuText);
    switch (sw)
    {
    case SW_DOWN:
        seq = seq ? seq - 1 : 0;
        break;
    case SW_UP:
        seq = (seq < sizeof(menu)/ sizeof(menu[0]) - 1) ? seq + 1 : seq;
        break;
    case SW_SELECT:
        if (menu[seq].menuFunc)
        {
            menu[seq].menuFunc();
        }
        seq = 0;
        break;
    }
}

void menu1()
{
    eeprom_write("TEST");
}
void menu2()
{
    eeprom_write("TEST2");
}

Comments

[SHOMI]

メニューについて書きましたが、500通り近くあるということは困っているのはメニューではなく、
状態の組み合わせでフラグやパターンが増えることでしょうか。
であればTaroToyotomiさんの回答にもあるようにステートマシンについて調べてみてください。

[dakao]

回答ありがとうございます。
調べてみます。

Answer 4

大量のcaseをど～するか、に限ると、
範囲ごと、あるいは機能ごとに関数にまとめてしまえがいいかと。

int SWITCH_A(int stat)
{
  switch(stat){
  case 0: break;  //  処理済み

  case STAT_1:
    なんやかや
    break;
  case STAT_2:
    なんやかや
    break;
  case いろいろ：
    ....


  default:
    return stat;
  }
  return 0;
}

というのを大量に作っておけば、

  stat=SWICH_A(stat);
  stat=SWICH_B(stat);
  stat=SWICH_C(stat);
  stat=SWICH_D(stat);
  stat=SWICH_E(stat);

ということができます

それと、提示のコードで気になるのは、caseの数値は直値を書くのは避けましょう
必ず、define あるいはenum でラベルを定義するようにします

Comments

[thkana]

メニューを一項目増やす度にあっちの関数こっちの関数をいじらなきゃいけない、というのは悪手だと思う...

[y_waiwai]

あくまで一つのcaseは特定の関数でしか処理しないってはなしなんで、そこらはやりようでどーとにもあるかと。

まあ、caseを何百も作るという自体が悪手なわけで、それをいきなり仕切り直しでステートマシンに実装し直しってのはちと非現実的なんで、まずは関数で細分化しよう、というおはなしでした。

[thkana]

そうですね...とにかく500は勘弁。

[dakao]

回答ありがとうございます。
このやり方なら自分でも簡単にできそうです。

[y_waiwai]

thkanaさんも懸念されてますが、ひとつのcaseは１箇所で処理する、というのを徹底させるようにしましょう。
これとこれは同じ処理だから、と複数の関数で分散させるようなことをするとすぐに破綻しますよ。