プログラムがどのように流れるかをよく考えると、かなりのメリットがあると思います。現在、あなたのプログラムフローから推測できるのは次のとおりです。
- 少なくとも 1 つのチャネル ID で、温度をチェックする外部ループがあります。そのループ内には、
if
最初に示したステートメントがあります。
- 次に、 activate alarm は他のことを行いますが、温度が下がるまでループし、 を呼び出し
logSubsystem
ます。
- logSubsystem はおそらく何らかのユーザー入力を取得し、そこから、おそらく prepare limit と呼ばれる初期関数を呼び出す必要があります。
これに関する問題は、これらの関数のいずれも完了しないことです。それらはすべて互いに呼び出し、最終的にはスタック オーバーフローが発生します。それがこのサイトの名前なのでいいのですが、あなたが熱望したいものではありません.
基本的に必要なのはステートマシンです。値を追跡し、それらの値を調べ、それらの値を操作する関数を呼び出すものが必要です。ループは 1 つだけで、それらの値に基づいて何が起こるかをすべて制御する必要があります。良いニュースは、これらすべてがすでに整っているということです。temperatureChannel
あなたのために値を追跡しており、whileループがたくさんあります。
あなたのプログラムの流れについて私が提案する方法について、私の提案をさせてください。
bool checkTemperatureValuesOutOfRange(int channelID) {
// this is just a convenience, to make the state machine flow easier.
return (temperatureChannel[channelID].currentTemperature > temperatureChannel[channelID].highLimit) || // note the || not just one |
(temperatureChannel[channelID].currentTemperature < temperatureChannel[channelID].lowLimit);
}
void actOnUserInput() {
char input = // ... something that gets a user input. It should check if any is available, otherwise return.
switch (input) {
case 'F':
case 'f':
temperatureChannel[channelID].currentTemperature--;
break; // This doesn't exit the loop - it gets you out of the switch statement
}
void activateAlarm(int channelID) {
// presumably this does something other than call logSubsystem?
// if that's all it does, just call it directly
// note - no loop here
logSubsystem(channelID);
}
void logSubsystem(int channelID) { // Not the current temperature - that's a local value, and you want to set the observed value
// I really think actOnUserInput should be (an early) part of the while loop below.
// It's just another input for the state machine, but I'll leave it here per your design
// Presumably actually logs things, too, otherwise it's an unnecessary function
actOnUserInput();
}
while (TRUE) { // this is the main loop of your function, and shouldn't exit unless the program does
// do anything else you need to - check other stuff
// maybe have a for loop going through different channelIDs?
if (checkTemperatureValuesOutOfRange(channelID)) {
activateAlarm(channelId);
// do anything else you need to
}
あなたのコードと私のコードの間に多くの違いがあることがわかると思います。考慮すべき重要事項を次に示します。
- すべての関数が返されます。マスター while ループは、ステータスをチェックする関数を呼び出し、ステータスを変更する関数を呼び出します。
- マスター while ループの一部として、ユーザー入力を処理することを強くお勧めします。これは、ステート マシンへの別の入力にすぎません。手に入れて行動し、自分のステータスを確認してください。おそらく、ユーザーからの入力が必要です。そうしないと、そもそも悪い状態になることはありません。
- 現在、アラームをアクティブにするたびに発生します。あなたが示したコードでは、問題ありません.logSubsystemが呼び出されていたからです。アラームを 1 回だけ鳴らしたい場合は、アラームが鳴ったかどうかを示すブール値トラッカーを temperatureChannel[channelId] 内に保持し、activateAlarm 内で true に設定し、checkTemperatureValuesOutOfRange の戻り値に基づいて false にリセットします。
- activateAlarm/logSubsystem 領域に自分自身を残すのではなく、毎回戻り、値を毎回チェックして、まだそこにいるかどうかを確認します。これが重要なポイントです。関数は高速でなければならず、プロセッサを独占してはなりません。各関数に 1 種類のことだけを実行させ、すべての制御をマスター ループ内から行うようにします。
私はあなたのコードに多くの変更を加えました。あなたがそれらすべてを行うことが許可されているかどうかはわかりませんが、これに似たものが必要になるでしょう. それははるかに堅牢であり、あらゆる場所で成長する余地があります.