switch文かif文か？

質問

スイッチ(SW50~SW80)を押したら、それに対応したコマンドを送る。というものを作っています。
スイッチはスライドスイッチなので、
1）ON(0)の間だけコマンドを出して、OFF(1)は止める。
2）ONしている間に別のスイッチが入ったら止める。
という風に組みたいのですが、中々上手くいきません。
8ピンのスライドスイッチなのですが、１～４はIDとして、５～８をコマンドとして使います。
なので、１～４は絶対にONになります。とすると、2）にしたいとき、defaultが使えなくて…if文を入れたらややこしくなりました。
もっと簡潔に、かつ解りやすく書くにはどうしたらいいでしょうか？

#define SW1 input(PIN_C0) // ID
#define SW2 input(PIN_C1) // ID
#define SW3 input(PIN_C2) // ID
#define SW4 input(PIN_C3) // ID
#define SW50 PIN_C4
#define SW60 PIN_C5
#define SW70 PIN_C6
#define SW80 PIN_C7

uchar LED_ON(void){
uchar DATA;
DATA=0x50;
DATA+=0x4F;
DATA+=0x4E;
return(DATA);
}

uchar LED_OFF(void){
uchar DATA;
DATA=0x50;
DATA+=0x4F;
DATA+=0x46;
return(DATA);
}

void LED(void){
ulong n;
uchar DATA;
while(1){
WriteFSKbyte(DATA);
switch(n){
case SW50:DATA=LED_ON();
if((SW50||SW60||SW80)==0){
DATA=LED_OFF();
} break;
case SW60:while(SW60==0){
DATA=LED_ON();
DelayMs(1000);
DATA=LED_OFF();
DelayMs(1000);
if((SW50||SW70||SW80)==0)
break;
} break;
case SW70:while(SW70==0){
DATA=LED_ON();
DelayMs(500);
DATA=LED_OFF();
DelayMs(500);
if((SW50||SW60||SW80)==0)
break;
} break;
case SW80:DATA=LED_OFF();
if((SW50||SW60||SW80)==0){
DATA=LED_OFF();
} break;
}
}
}

回答No.7

mtaka2 · ベストアンサー

> sw1234=input_c();
これだと、sw1234には、SW5～SW8のデータも入ってしまいます。
> sw1234=input_c() & 0x0f;
とすることで、下位4bit=SW1～SW4のデータだけが入れられます。

あと、スイッチと入力値の対応が「ON=0、OFF=1」だとのことですから、
> case 0x01:WriteFSKbyte(0x01);break;
これだと逆に「SW1がOFFでSW2～4がONの時」って条件になってしまいます。
SW1だけがONな場合の条件は、SW1だけ0でSW2～SW4を1にした「case 0x0E:」になります。

で、話を戻しますが

> SW１～４のどれかが入っていないと、SW５～８は出力しない

この条件は「どれか」でいいのでしょうか?
SW1～SW4が二つ以上入ってもかまわないが、
「SW1～SW4が全てOFFの場合」(sw1234が0x0Fの時)は処理しない
でいいのだったら、

　　while (1) {
　　　　sw1234 = input_c() & 0x0f;
　　　　sw5678 = input_c() & 0xf0;
　　　　switch(sw5678) {
　　　　case 0xe0: /* SW5～SW8 のうち、SW5だけON */
　　　　　　if (sw1234 != 0x0F) LED_ON();
　　　　　　break;
　　　　case 0xd0: /* SW5～SW8 のうち、SW6だけON */
　　　　　　if (sw1234 != 0x0F) LED_ON();
　　　　　　break;
　　　　…

と記述できます。

SW1～SW4のうちでも「一つだけしかONにしてはいけない」だったら、

> SW1(ON)＋SW5(ON)＝LED_ON
> SW2(ON)＋SW5(ON)＝LED_ON
> SW1(ON)＋SW8(OFF)＝LED_OFF

こういう条件を全てリストアップしたうえで、
以下のように、複数のスイッチの状態をまとめて判断するという方法もあります。
(switch文では、break せずに case を複数並べることで、複数の条件に対し処理をまとめることができます)

　　int sw;
　　while(1){
　　　　sw = input_c();
　　　　switch(sw){
　　　　case 0xEE: /*二進11101110、SW1(ON)+SW5(ON) */
　　　　case 0xED: /*二進11101101、SW2(ON)+SW5(ON) */
　　　　case 0xEB: /*二進11101011、SW3(ON)+SW5(ON) */
　　　　case 0xE7: /*二進11100111、SW4(ON)+SW5(ON) */
　　　　　　/* SW1～SW4のうちどれか1つだけONで、SW5～SW8 のうちSW5だけがONの場合 */
　　　　　　LED_ON();
　　　　　　break;
　　　　case 0xDE: /*二進11011110、SW1(ON)+SW6(ON) */
　　　　case 0xDD: /*二進11011101、SW2(ON)+SW6(ON) */
　　　　case 0xDB: /*二進11011011、SW3(ON)+SW6(ON) */
　　　　case 0xD7: /*二進11010111、SW4(ON)+SW6(ON) */
　　　　　　/* SW1～SW4のうちどれか1つだけONで、SW5～SW8 のうちSW6だけがONの場合 */
　　　　　　LED_ON();
　　　　　　break;
　　　　…

お礼

再三にわたり、ありがとうございます。

> case 0x01:WriteFSKbyte(0x01);break;
の部分、書き込みした後で気づきました…これじゃ逆でしたね。

ＳＷ１～４ですが、
・ＳＷ１のみＯＮの時～ＳＷ１-４がＯＮの時
までの１５通り（全ＯＦＦも含めれば１６ですね）がありますので、最初に提示して頂いたサンプルがぴったりでした。
ＳＷ１２３４をいれてからという概念が強すぎたので、ＳＷ５６７８の中に入れる事を考えていませんでした。勉強になります。
以下、実機で動かしたものになります。サンプルそのままですが…
void LED(void){
　　int sw1234,sw5678;
　　set_tris_a(0x00);
　　set_tris_c(0xFF);
　　set_tris_d(0xFF);
　　output_low(PIN_A0);
　　while(1){
　　　　sw1234=input_c()&0x0F;
　　　　sw5678=input_c()&0xF0;
　　　　switch(sw5678){
　　　　　　case 0xE0:
　　　　　　　　if(sw1234 != 0x0F)
　　　　　　　　output_high(PIN_A0);
　　　　　　　　break;
　　　　　　～～　略　～～
　　　　　　case 0x70:
　　　　　　　　if(sw1234 != 0x0F)
　　　　　　　　output_low(PIN_A0);
　　　　　　　　break;
　　　　　　default:
　　　　　　　　output_low(PIN_A0);
　　　　　　　　break;
　　　　}
　　}
}
output(PIN_A0)の出力が、設計上の都合で
low(0)＝OFF、high(1)＝ON
になっています。

回答No.6

mtaka2

たぶん PIN_C0 とかの表記はPIC用のCコンパイラの一つ、 CCS-C の独自の表記ですね。
そのあたりの状況説明無しでは話が見えません。

あと、行き当たりばったりに書くのではなく、C言語の仕様とPICの制御をちゃんと把握してコードを書かないといけません。

○switch 文の構文について
switch文の構文では、
> switch(n){
> case SW50: …(a)… break;
> case SW60: …(b)… break;
> …
> }

という表記は、
> if (n == SW50) {
> 　　…(a)…
> } else if (n == SW60) {
> 　　…(a)…
> } else …

という表記と同じ意味です。
質問者さんのコードでは、変数nに何の値も代入してないため、意味のある分岐になりません。
あと、ここで case に指定する値は「定数」でなければいけませんが、
だからといって、SW50 とかを入れてますけど、それではポートの入力チェックにもなりません(後述)

○SW50～SW70(PIN_C4～PIN_C7)
これらは(おそらく)「ピン番号」を意味する「定数」です。ですから、
> if((SW50||SW60||SW80)==0){
> while(SW60==0){
これらのコードは、ポートの入力値の調査はまったく行わない「定数の比較」です。
0でない定数を0と比較してますから、if文が実行されることはありませんし、whileループもまったく回りません。

○LED_ON / LED_OFF 関数
> uchar LED_ON(void){
> uchar DATA;
> DATA=0x50;
> DATA+=0x4F;
> DATA+=0x4E;
> return(DATA);
> }
この関数は、
> uchar LED_ON(void) {
> return 0xED;
> }
と等価です。(0xED= 0x50 + 0x4F + 0x4E)
単にそういう計算を行っているだけで、ポートの入出力は何もしていません。

また、変数 DATA に代入した値は、全体の while ループの冒頭で1度だけ WriteFSKbyte 呼び出しに使っているだけですので、
その中の 変数 DATA への代入は、大半が意味のないものになってます。

推測ですが、おそらく
> void LED_ON(void){
> 　　WriteFSKbyte(0x50);
> 　　WriteFSKbyte(0x4F);
> 　　WriteFSKbyte(0x4E);
> }
というのが、お望みのコードじゃないですか?

○複数のスイッチの状態をまとめて処理する方法

以上のことを踏まえて、多分に推測が入ってますが、質問者さんのコードの意図を、
私の最初の回答に述べたようなswitch の使い方で、CCS-C 向けに書き直すと、

void LED_ON(void){
　　WriteFSKbyte(0x50);
　　WriteFSKbyte(0x4F);
　　WriteFSKbyte(0x4E);
}
void LED_OFF(void){
　　WriteFSKbyte(0x50);
　　WriteFSKbyte(0x4F);
　　WriteFSKbyte(0x46);
}
void LED(void) {
　　int sw5678;
　　while (1) {
　　　　sw5678 = input_c() & 0xf0;
　　　　/*0xf0は2進数で 11110000b、ポートCの上位4bitのみ取り出し (sw5678 の上位4bitはポート入力値そのままで、sw5678の下位4bitは0になる)*/
　　　　switch(sw5678) {
　　　　case 0xe0:
　　　　　　/* 2進数で1110000b 、上位4bit中5bit目だけが0 =SW5～SW8のうち、SW5だけがオンの時の処理をここに*/
　　　　　　LED_ON();
　　　　case 0xd0:
　　　　　　/* 2進数で1101000b 、上位4bit中6bit目だけが0 =SW5～SW8のうち、SW6だけがオンの時の処理をここに*/
　　　　　　LED_ON();
　　　　　　DelayMs(1000);
　　　　　　LED_OFF();
　　　　　　DelayMs(1000);
　　　　case 0xb0:
　　　　　　/* 2進数で1011000b 、上位4bit中7bit目だけが0 =SW5～SW8のうち、SW7だけがオンの時の処理をここに*/
　　　　　　LED_ON();
　　　　　　DelayMs(500);
　　　　　　LED_OFF();
　　　　　　DelayMs(500);
　　　　case 0x70:
　　　　　　/* 2進数で0111000b 、上位4bit中8bit目だけが0 =SW5～SW8のうち、SW8だけがオンの時の処理をここに*/
　　　　　　LED_OFF();
　　　　default:
　　　　　　/* スイッチが全部オフか、2つ以上がオンの場合 の処理をここに*/
　　　　　　LED_OFF();
　　　　}
　　}
}

といった感じになるんじゃないでしょうか。

お礼

再度ありがとうございます。
C言語を始めて日が浅いのですが、入りがccs-cなのでこれが当たり前のものだと認識していました…申し訳ありません。
知識も経験も無い中で、早く組まねばという焦りだけできてしまっているので、意味の無いモノが出来上がってしまったと痛感しています。御忠告ありがとうございます。もっとよく考えようと思います。
そんな中で、的確に答えて頂いてとてもありがたいです。
if文・switch文にそのような相互関係があると初めて理解しました。凄く解りやすく、助かります。

LED_ON、_OFFの関数ですが、まさにその通りだと思われます。
transmitterとreceiverでタイミングを合わせないとわかりませんが、サンプルに近いのでいけるのでは、と。

LED関数も、少し変えて(WriteFSKbyteがまだ使えないので、output_low・high(PIN)で代用して)実機で動かしてみました。
動かして、プログラムの内容が理解できました。

SW1(ON)＋SW5(ON)＝LED_ON
SW2(ON)＋SW5(ON)＝LED_ON
SW1(ON)＋SW8(OFF)＝LED_OFF
・・・・・・
SW1～4(全OFF)＋SW5～8(どれかON)＝出力なし
というふうになるんですが、SW１～４のどれかが入っていないと、SW５～８は出力しないようにしたいのです。とすると、＞上位4bitのみ取り出しという部分は使えなくなりますよね？11110000bだと、下位4bitは入力では認識されないということで合っていますか？

補足

ちなみに、提示していただいたサンプルを使って、別体で考えるとsw1～4はこうなったのですが…

void Switch_C(void){
　　int sw1234;
　　　　while(1){
　　　　　　sw1234=input_c();
　　　　　　switch(sw1234){
　　　　　　case 0x01:WriteFSKbyte(0x01);break;
　　　　　　case 0x02:WriteFSKbyte(0x02);break;
　　　～～　略　～～
　　　　　　case 0x0F:WriteFSKbyte(0x0F);break;
　　　　　　default:break;
　　　　}
　　}
}

回答No.5

tsuduki999

ハードウェアとか組み込み系はよくわからないので、どう難しいのかわからないのですが、
素直にスィッチ一つずつを関数にして強制的に実行するようにしてみてはだめなのですか？
スイッチ一つを一つの機能だと考えて設計すると幸せになれそうな気がするのですが。。。

僕のイメージはこんな感じです。
void LED()
{
　　uchar DATA;

　　void　send_SWnn()
　　{
　　　　if(　ON判定　)
　　　　{
　　　　　　DATA=LED_ON();
　　　　　　DelayMs(500);
　　　　　　DATA=LED_OFF();
　　　　　　DelayMs(500);
　　　　}
　　}

　　send_SW50();
　　send_SW60();
　　send_SW70();
　　send_SW80();
}

上はコマンドを送信するというフローイメージがわかないので、
とりあえず、順番に垂れ流しですが、その辺は調整してください。

お礼

回答ありがとうございます。
提示していただいたサンプルを使うとすると、
send_SWnn()がスイッチ一つ一つの関数になり、それを組み合わせたときの動作をそれぞれ決める。
という事で合っていますか？

回答No.4

SilverThaw

No.1です。

>試しに書いてみたのですが、このような書き方でよいでしょうか？
一つ目(外側)のswitch文と二つ目(内側)のcaseが同じ定義を使用しているようですが、
外側は「今現在の『状態』」で内側は「発生したイベント（この質問ならキーのON/OFF？）」になりますから、同じ定義はそのまま使用することはできないと思います。
また、「キーの同時押し」というイベントも考えられますよ。

お礼

再度ありがとうございます。
同じ定義を使用…とは、どの部分を指しているのでしょうか？
同時押しも考えてみたのですが、私はどうも考えが固着してしまっているようで、これ以外にわかりません…

回答No.3

mtaka2

LED関数内でのnへの代入がどこにもないし、
SW50～SW80の定義がよくわからない(switchのcaseには定数しか使えませんが、if文中ではポートの入力データかのように使ってる)ので、
元のコードの意味が読み取れないというか、私が取り違えてるかもしれませんが、

ポートからの入力で、8bitのうち上位4bitを見たい。下位4bitは無視したい。ということだったら、
擬似コードで書くと、

int data;
while (1) {
　　data = ポート入力値 & 0xf0; /*0xf0は2進数で 11110000b、上位4bitのみ取り出し (data の上位4bitはポート入力値そのままで、下位4bitは0になる)*/
　　switch(data) {
　　case 0xe0: /* 2進数で1110000b 、上位4bit中5bit目だけが0 =5bit目のスイッチだけがオン*/
　　　　…
　　case 0xd0: /* 2進数で1101000b 、上位4bit中6bit目だけが0 =6bit目のスイッチだけがオン*/
　　　　…
　　case 0xb0: /* 2進数で1011000b 、上位4bit中7bit目だけが0 =7bit目のスイッチだけがオン*/
　　　　…
　　case 0x70: /* 2進数で0111000b 、上位4bit中8bit目だけが0 =8bit目のスイッチだけがオン*/
　　　　…
　　default: /* スイッチが全部オフか、2つ以上がオンの場合 */
　　　　…
　　}
}

といった感じで処理を簡潔化できます。

お礼

回答ありがとうございます。
ulong ｎ;
はswitch(n)として使っているのですが、それだけでは駄目でしょうか？どこかに出力させた方がいいのでしょうか？

SW50～SW80
については、PICを使っていまして、PICのポートCの４PINから７PINを割り当てています。最初はSW1～SW8(input付)・SW10～SW80(PINのみ)まであったのですが、使わないので省略しました。
PIN番号で書くとcaseに使えないし、他の場所でもSWが出てくるので、解りやすくするために定義しています。
下位４bit(ID)につてですが、例えば
SW1(ID側)のみONの時、かつSW5(コマンド側)のみがONの時、SW5に割り当てられた動作をする。
ということで、ID側は全てが常にONになっているわけではなく、1つずつ使いたいのです。

説明も理解も足らず、申し訳ありません。

回答No.2

wathavy

スイッチの状態と、そのスイッチ自身の変化をスイッチ読み込みに使おうとすると、非常に難しいと思います。

１個だけ、トリガーとして、ON　のときの立ち上がりで、（あるいはOFFへの立下り）他のスイッチを読み込み、それを変数（ビット数がわからないので答えにくいのですが）に入れて、スイッチ文で処理するとすっきりしませんか？

お礼

回答ありがとうございます。
実は、これは送受信無線（SPI通信）で使うものなのですが、その時にtransmitter側から送る一部なんです。
無線で送るときにクロックの立下りでデータ送信しているのですが、実のところよく理解できてません。

＞一個だけトリガとして～
というのは、どういったことなのでしょうか？
理解が足らず、申し訳ありません…

回答No.1

SilverThaw

>もっと簡潔に、
「簡潔」＝「簡単」＝「ソースを短く」には無理です。

>かつ解りやすく書くにはどうしたらいいでしょうか？
一例として「ステートマシン」という手法があります。
何らかの動作を行っている状態に対してそれぞれ番号を振り管理する方法です。

質問文に当てはめれば
各SWが「押されている状態」、「押されていない状態」をそれぞれ「状態（ステート）」とします。
SWの入力状態に変化があった場合を「イベント」とします。
それぞれの状態に対して、どのイベントが発生したかによって処理を分けることによって実現する方法です。
但し、構造自体は大きくなります。

例）
switch( 現在の状態 )
{
　case SW5_ON中:
　　　switch( 発生したイベント )
　　　{
　　　　case SW_6が押された
　　　　　　送信の停止
　　　　　　状態を停止状態にする
　　　　　break;

　　　　case SW_8が押された
　　　　　break;
　　　}　
　　　break;

　case SW6_ON中:
　　・
　　・
}

お礼

回答ありがとうございます。
試しに書いてみたのですが、このような書き方でよいでしょうか？
エラーにはなりませんでした。
while(1){
　　　WriteFSKbyte(DATA);
　　　　　switch(sw){
　　　　　　　case SW50:
　　　　　　　　　　DATA=LED_ON();
　　　　　　　　　　switch(asw){
　　　　　　　　　　　　　case SW60:
　　　　　　　　　　　　　case SW70:
　　　　　　　　　　　　　case SW80:DATA=LED_OFF();break;
　　　　　　　　　　}
　　　　　　　　　　break;
　　　　　　　case SW60:
　　　　　　　　　　while(SW60==0){
　　　　　　　　　　　　　DATA=LED_ON();
　　　　　　　　　　　　　DelayMs(1000);
　　　　　　　　　　　　　DATA=LED_OFF();
　　　　　　　　　　　　　DelayMs(1000);
　　　　　　　　　　　　　switch(asw){
　　　　　　　　　　　　　　　　case SW50:
　　　　　　　　　　　　　　　　case SW70:
　　　　　　　　　　　　　　　　case SW80:DATA=LED_OFF();break;
　　　　　　　　　　　　　}
以下続く…