Difficulty in starting all engine types consistently

[seanmcleod70]

I added an example Python script to the repo a while ago to generate a trim envelope. It defaults to using the 737 model in the repo but should be applicable to any aircraft, ideally by simply changing the aircraft type in the script.

jsbsim/examples/python/TrimEnvelope.py

Line 30 in 2006648

AIRCRAFT_NAME="737"

One of the first things it needs to do is to ensure that all the engines of the aircraft are running before asking for trim solutions. This is currently how it starts the engines.

jsbsim/examples/python/TrimEnvelope.py

Lines 42 to 44 in 2006648

	# Set engines running
	fdm['propulsion/engine[0]/set-running'] = 1
	fdm['propulsion/engine[1]/set-running'] = 1

There are 2 things that aren't ideal in terms of this engine start procedure. It needs to be edited based on the number of engines, and secondly it fails for piston engines.

The property propulsion/engine[x]/set-running ends up simply calling:

jsbsim/src/models/propulsion/FGEngine.h

Line 158 in 2006648

virtual void SetRunning(bool bb) { Running=bb; }

Which is fine for turbine engines but not for piston engines.

So I switched to using:

fdm['propulsion/set-running'] = -1

Which is better since you can pass -1 to specify all engines and it ends up calling into engine type specific InitRunning() methods, e.g.

jsbsim/src/models/propulsion/FGPiston.cpp

Lines 540 to 548 in 2006648

	int FGPiston::InitRunning(void)
	{
	Magnetos=3;
	in.MixtureCmd[EngineNumber] = in.PressureRatio*1.3;
	in.MixturePos[EngineNumber] = in.PressureRatio*1.3;
	Thruster->SetRPM( 2.0*IdleRPM/Thruster->GetGearRatio() );
	Running = true;
	return 1;
	}

But in testing this with piston engine aircraft I noticed an issue between the C172x model and the C172p model. For the C172x model it worked as expected, but for the C172p model it failed, even though they use the exact same engine.

I tracked the difference down to the C172x having an automatic mixture control which the C172p model doesn't have.

jsbsim/aircraft/c172x/c172x.xml

Lines 305 to 328 in 2006648

	<system name="Mixture control">
	<!--
	E = 1.3 * Mixture * P_std / P_amb
	Mixture = (P_amb / P_std) / 1.3
	-->
	<channel name="Automatic Mixture Control">
	<fcs_function name="systems/mixture-cmd-norm">
	<function>
	<quotient>
	<table>
	<independentVar lookup="row">atmosphere/P-psf</independentVar>
	<tableData>
	0 0.0
	2117 1.0
	</tableData>
	</table>
	<value>1.0</value>
	</quotient>
	</function>
	<output>fcs/mixture-cmd-norm</output>
	</fcs_function>
	</channel>
	</system>

I could get the C172p engine to run with this combination in the script:

fdm['propulsion/set-running'] = -1
fdm['fcs/mixture-cmd-norm'] = 1

My guess at the moment, will need to debug, is that when FGPiston::InitRunning() is called in.PressureRatio is 0 or unitialised which results in mixture command setting that prevents the engine from running?

[seanmcleod70]

Hmm, so it turns out FGPropulsion::InitRunning() actually sets the mixture command and position before calling the engine type specific InitRunning().

jsbsim/src/models/FGPropulsion.cpp

Lines 332 to 341 in 2006648

	} else if (n < 0) { // -1 refers to "All Engines"
	for (unsigned int i=0; i<GetNumEngines(); i++) {
	in.ThrottleCmd[i] = in.ThrottlePos[i] = 1; // Set the throttle command and position
	in.MixtureCmd[i] = in.MixturePos[i] = 1; // Set the mixture command and position
	GetEngine(i)->InitRunning();
	}
	GetSteadyState();
	}

Plus, there is no issue with in.PressureRatio inside FGPiston::InitRunning().

What seems to happen is that after InitRunning() has been called, with the mixture command = 1 that subsequently during the simulation timesteps the FCS is queried for the mixture command, which returns 0 since it hasn't been set and in the case of the C172p there is no system to set it during each timestep. Then with a value of 0 it results in 0 fuel flow.

So I guess the question is whether instead of setting in.MixtureCmd[i] = in.MixturePos[i] = 1 in FGPropulsion::InitRunning() it should rather be setting them via the FCS? That way they will persist for aircraft/scripts that don't explicitly set the mixture command, and for aircraft/scripts like the C172x the FCS value will be updated on the next timestep.

[seanmcleod70]

So as a quick test, this does fix the issue with the C172p.

    for (unsigned int i=0; i<GetNumEngines(); i++) {
      in.ThrottleCmd[i] = in.ThrottlePos[i] = 1; // Set the throttle command and position
      in.MixtureCmd[i] = in.MixturePos[i] = 1;   // Set the mixture command and position
      FDMExec->GetFCS()->SetMixturePos(i, 1);    // Also set FCS values     
      FDMExec->GetFCS()->SetMixtureCmd(i, 1);
      GetEngine(i)->InitRunning();
    }

Also confirmed it doesn't cause an issue with the C172x, by reporting the mixture command value after InitRunning(), it's the system calculated value. Lastly for C172p case, if a user does manually specify a mixture command like in the example below, I also confirmed that this value is in use after InitRunning().

  <use aircraft="c172p" initialize="reset02"/>
  <run start="0.0" end="20" dt="0.00833333">

	<property value="-1"> propulsion/set-running </property>
	<property value="0.8"> fcs/mixture-cmd-norm </property>

Any comments, suggestions before I submit a pull request along these lines?

[bcoconni]

Good catch ! Yes, I have a few questions:

1. Do we still need to call in.MixtureCmd[i] = in.MixturePos[i] = 1; ? Isn't this line redundant with the calls to FDMExec->GetFCS()->SetMixture[...] ?
2. I'm wondering why we need to set both mixture-cmd-norm and mixture-pos-norm ? The question also holds for the previous design BTW. Theoretically, mixture-pos-norm is set by mixture-cmd-norm, either by default or by the user's FCS (see the code below: first MixturePos is forced to the same value than MixtureCmd, then the FCS aka SystemChannels is run where the user might overwrite the value of MixturePos that has just been set)
   

   jsbsim/src/models/FGFCS.cpp

   Lines 164 to 176 in 50ed574

   	for (i=0; i<MixturePos.size(); i++) MixturePos[i] = MixtureCmd[i];
   	for (i=0; i<PropAdvance.size(); i++) PropAdvance[i] = PropAdvanceCmd[i];
   	for (i=0; i<PropFeather.size(); i++) PropFeather[i] = PropFeatherCmd[i];
   	// Execute system channels in order
   	for (i=0; i<SystemChannels.size(); i++) {
   	if (debug_lvl & 4) {
   	FGLogging log(FDMExec->GetLogger(), LogLevel::DEBUG);
   	log << " Executing System Channel: " << SystemChannels[i]->GetName() << endl;
   	}
   	ChannelRate = SystemChannels[i]->GetRate();
   	SystemChannels[i]->Execute();
   	}

3. Why should your change be limited to the "all engines" option ? Shouldn't it be applied to the case where FGPropulsion::InitRunning is called on a specific engine ?
   

   jsbsim/src/models/FGPropulsion.cpp

   Lines 326 to 330 in 50ed574

   	in.ThrottleCmd[n] = in.ThrottlePos[n] = 1; // Set the throttle command and position
   	in.MixtureCmd[n] = in.MixturePos[n] = 1; // Set the mixture command and position
   	GetEngine(n)->InitRunning();
   	GetSteadyState();

4. I'm not sure to understand the following statement, could you please elaborate ?

Lastly for C172p case, if a user does manually specify a mixture command like in the example below, I also confirmed that this value is in use after InitRunning().

[seanmcleod70]

1. I'll double-check, but I'm pretty sure the issue was that FGPropulsion::InitRunning() calls into the engine type specific, e.g. FGPiston::InitRunning() directly before there is another call to FGFCS::Run, and FGPiston::InitRunning() uses in.Mixturexxx directly, i.e. doesn't query the FCS for the mixture command/position.

2. Think it's the same issue as 1, i.e. the FCS doesn't get a chance to run to map MixtureCmd to MixturePos.

3. It isn't, I have the same code for non-all engines case, I only showed the "all engines" code snippet to keep the snippet smaller in my discussion comment.

4. Yep, wasn't explained very clearly. What I was wanting make sure of was that injecting the mixture cmd/pos of 1 via the FCS via my proposed code change for the InitRunning() call wouldn't override any subsequent user defined property sets of the mixture command.

So say the user had a script along these lines for the C172p (no system element for automatically controlling the mixture), then when propulsion/set-running is called the FCS value of mixture cmd/pos would be set to 1 within the InitRunning() function, but immediately after that on the next timestep of the simulation the FCS value would be the user supplied value of 0.8, which would then also be in force during the trim event etc.

The InitRunning() function will also be called if the user supplies an initialization file, e.g. reset02.xml with a <running/> element.

  <use aircraft="c172p" initialize="reset02"/>
  <run start="0.0" end="20" dt="0.00833333">

	<property value="-1"> propulsion/set-running </property>
	<property value="0.8"> fcs/mixture-cmd-norm </property>

       <event name="Trim">
            <condition>simulation/sim-time-sec  ge  0.50</condition>
            <set name="simulation/do_simple_trim" value="1"/>

Thinking about it now in typing this up, there is the issue that if the user swaps the order around to:

	<property value="0.8"> fcs/mixture-cmd-norm </property>
	<property value="-1"> propulsion/set-running </property>

Then the user supplied mixture command will be lost/overwritten to 1.

So maybe I should modify the code to query the current FCS value for the mixture and if it's non-zero (can't tell if user set it to 0) then reset the FCS to this value after calling the engine specific InitRunning.

[bcoconni]

Thanks for your answers, that's clear to me now.

So maybe I should modify the code to query the current FCS value for the mixture and if it's non-zero (can't tell if user set it to 0) then reset the FCS to this value after calling the engine specific InitRunning.

I would refrain from doing that. The order in which the properties are set is meaningful in JSBSim¹ so I would suggest we keep it this way.

Footnotes

1. Well, this statement must be true in 99% of the cases. There are most likely some edge cases where it is not but generally speaking the order is meaningful. ↩

[seanmcleod70]

Okay I've refrained from adding the additional logic to check the FCS value first etc.

Created a pull request - #1266

[bcoconni]

The PR is pushed to master. Thanks for finding and fixing this error 👍