c172p-team / c172p

A high detailed version of the Cessna 172P aircraft for FlightGear
GNU General Public License v2.0
82 stars 43 forks source link

Bug with fuel priority #1371

Closed wlbragg closed 3 years ago

wlbragg commented 3 years ago

I introduced a bug back in commit cba9e7a3 ("Add integral tank option", 2020-08-16).

The fix was to refactor some JSBSim logic. I really don't understand why the logic didn't work, but the new logic does. If anyone knows why this logic doesn't work I would like to know.

Old logic (broken)

    <channel name="Float Chamber">
        <!-- Give tank[4] priority regardless of engine is used -->
        <switch name="Float Chamber">
            <output>propulsion/tank[4]/priority</output>
             <default value="0"/>
            <test logic="OR" value="1">
                 /controls/engines/active-engine EQ 0
                 /controls/engines/active-engine EQ 1
             </test>
             <test logic="AND" value="1">
                 /engines/active-engine/killed EQ 0
             </test>
         </switch>

New logic (fix)

    <channel name="Float Chamber">
        <!-- Give tank[4] priority regardless of engine is used -->
        <switch name="Float Chamber From Engine 0">
            <output>propulsion/tank[4]/fuel/to-engine0</output>
            <default value="0"/>
            <test logic="AND" value="1">
                /controls/engines/active-engine EQ 0
                /engines/active-engine/killed EQ 0
            </test>
        </switch>
        <switch name="Float Chamber From Engine 1">
            <output>propulsion/tank[4]/fuel/to-engine1</output>
            <default value="0"/>
            <test logic="AND" value="1">
                /controls/engines/active-engine EQ 1
                /engines/active-engine/killed EQ 0
            </test>
        </switch>
        <switch name="Float Chamber">
            <output>propulsion/tank[4]/priority</output>
            <default value="0"/>
            <test logic="OR" value="1">
                propulsion/tank[4]/fuel/to-engine0 EQ 1
                propulsion/tank[4]/fuel/to-engine1 EQ 1
            </test>
        </switch>
    </channel>
legoboyvdlp commented 3 years ago
<test logic="OR" value="1">
                 /controls/engines/active-engine EQ 0
                 /controls/engines/active-engine EQ 1
             </test>

That is always going to be true, isn't it? Because active engine will either be 0 or 1. Therefore tank 4 will have priority 1 regardless of whether the engine is killed or not.

wlbragg commented 3 years ago

@legoboyvdlp Right, except the switch should be calculated as a whole including the second set section

             <test logic="AND" value="1">
                 /engines/active-engine/killed EQ 0
             </test>

So when /engines/active-engine/killed = 1 the switch should set to default = 0 which should make propulsion/tank[4]/priority = 0 no mater which tank is selected? It'd done and it works now so I am OK with it, but i really want to know why the other didn't work? Unless I don't understand how the switch statement works. I assumed the second set section overrides the first.

legoboyvdlp commented 3 years ago

Nope, it doesn't work that way. The second won't override the first, the first one overrides the second and if the first one is true, then it doesn't matter if the second one is true or false.

wkitty42 commented 3 years ago

sounds like what we call "boolean short circuit" where in a multipart boolean statement, once true is found, we exit out of the statement without evaluating any more of the statement...