Fix quadrants by removing active engines

[stuartbuchanan]

Hi Folks,

There's been a long-standing bug with the c172p that hardware throttle quadrants don't work. The root cause is the model having multiple engine definitions and logic to select an active-engine.

On balance I think being able to use a throttle quadrant is more important than being able to change the engine in-sim, and I finally got around to looking at fixing it. I wasn't able to find a way to do so by over-riding the controls.nas module (which has previously caused problems in itself), so spit the FDM file in two based on the engine type (io320 or io360).

The downside is that it's not possible to change the engine type in-sim. However, I think selecting it in the launcher makes sense and having the amphibious and float planes use the io360 makes sense.

The change below is largely mechanical - removing /engines/active-engine.

-Stuart

[onox]

Nice to see this clean up. It's a bit unfortunate though that JSBSim still doesn't support defining multiple engine variants, which means there are now 2 FDMs that need to kept in sync.

1. In the list of changes I see you have removed the 'Start with engine running' option. Is there a specific reason for that?
2. Shouldn't the capability to change the variant in-sim be removed as well? Otherwise when a user switches to the float variant, the engine may still be 160hp.

[wlbragg]

On balance I think being able to use a throttle quadrant is more important than being able to change the engine in-sim,

Of the top of my head I'm not sure this is the correct fix. I think there is more tied to these multiple engines than just the throttle. There has to be a way to make the throttle quadrant work with either engine. I want to look at this a little closer and see exactly what the issue is. I have heard this issue before and don't fully understand the details of the issue. I have a throttle quadrant and it works fine. Is that because I mapped it differently than the default? What make us think we can't map both engines to the [zero] engine throttle using filters or code of some sort.
I'm not saying it is flat out wrong to separate them and I am not opposed entirely to making the engines a variant, but there is a lot of code tied to this two engine regime. If the downside is that a JS mapping is having issues, maybe it is better to look at the JS mappings and/or the JSBSim portion if the issue is JSBSim doesn't support multiple engines.
I feel I need more information and also need to look over the proposed changes before I am onboard with this.

What's up with the state overlay stuff. Is this new state overlay definitions or maybe old definitions that we had removed?

[wlbragg]

@stuartbuchanan is this just the number 0 engine accounted for or is the number 1 engine also part of this code as variant? In other words, does the variant, larger engine, still need to be duplicated from these changes. All I saw was engine 0?

[wlbragg]

I have a throttle quadrant and it works fine. Is that because I mapped it differently than the default?

Why is it so hard or not appropriate to change the bindings for the joysticks? How are aircraft with multiple engines that are operated simultaneously managed? If the only argument is that the majority of aircraft have always used engines/engine[0]/xxx or all the nasal bindings uses that property, why don't we just fix those bindings so there is a "common" or universal property that can be used by all to get any current or future single or multiple engine configuration to work right out of the box?
I have less of an argument with a single engine aircraft such as the Cub or Cessna being configured as one aircraft with on the fly engine switching as I do with multi-engine aircraft needing bindings for all the engines. Maybe we missed the mark on using code that is implemented for multi-engine aircraft JS bindings? As in maybe we could have done this differently and used a "common" or "accepted" way to bind engines past [0]? VS creating our own universal binding of engines/active-engine/xx
I feel that property is a good way to handle this. If it had been adopted from the start, then this wouldn't be an issue, correct?
I'm just having a really hard time wrapping my head around why it is necessary to change the code in a couple aircraft, or more, to make up for or accommodate less than robust joystick bindings that are old and no longer supportive of the capabilities we have introduced into the more current aircraft? To my way of thinking it is "settling" for a less than perfect solution and taking a step backwards.

[stuartbuchanan]

Hi Folks,

Thanks very much for the feedback. Attempting to collate all the feedback so far:

@onox - I removed the "start with engine running" option as it's covered by support for the state system (which is the next commit I'd like to get pulled). I thought I'd removed the option to change the engine within the engine options dialog?

@wlbragg

• I've for another pull request that also sorts out the state overlays. It's not perfect (for example JSBSim isn't trimming properly) but it is robust and means that you will start the aircraft with the engine running on a runway (which will address a lot of new-user problems who struggle the start the engine), and with the aircraft tied up properly and cold-and-dark if you start from the parking. I'd like to get both of these into 2020.2
• With this change there is no-longer a number 1 engine (/engines/engine[1]/). Instead the -set file only loads a single engine definition. So you just have /engines/engine[0]/ and all the normal controls Just Work. The amphibious and float variants use the io360, while the other variants use the io320. It might be worth changing the bush variant to use the io360 as well, if that's a common configuration.
• Joystick quadrant control mappings end up mapping to /controls/engines/engine[x]/[throttle|mixture|prop]. There are various ways to do this (see Nasal/controls.nas), but by default a quadrant with two throttle levers will set /controls/engines/engine[0]/throttle and /controls/engines/engine[1]/throttle. Interesting that your throttle is working. I suspect that if you just have a single throttle lever you may be setting /controls/engines/throttle-all, which will end up setting /controls/engines/active-engine/throttle because the code simply sets /controls/enginers/*/throttle. However that doesn't work for quadrants with multiple throttle levers.
• You asked about the io360 variants. There is a second fdm file (c172p_io360.xml) which contains the second FDM variant with the bigger engine. It's used by c172p-amphibious-set.xml and c172p-float-set.xml. Obviously we could also use it for the big bush-wheeled variants as well if we wanted to.
• You mention that "there's a lot of code tied to this engine regime". That's correct, in so far that there was a lot of code and filters which had to accomodate the support for in-sim engine selection, but didn't actually add anything functionally. As you'll see, I've managed to remove all of it. Paring it all back, The only bit of interest is that there are different engine files, oil tank capacity and engine masses. All of this I've handled in the -set and FDM configuration. All the rest was basically hacks to make the engine selection work.
• You ask how aircraft with multiple engines that are operated simultaneously managed. You can configure a joystick axis to affect multiple engines (see controls.nas perEngineSelectedAxisHandler, /controls/engines/throttle-all), so you can have one lever controlling the port side engines, and the other controlling the starboard. Additionally, there is code to select engines for the keyboard throttle.

I don't think the problem is that the joystick bindings aren't robust. I think the fundamental issue here is that the 172 is not a twin engined aircraft :) , and VS had to do a huge number of hacks to try to fool the FDM and simulator into thinking that what at a configuration level had two engines, in fact only had one Basically, this breaks the simulation "model".

The correct way to handle this is through variants and having separate FDM files to handle airframes with substantially different FDMs. That's what this fixes.

[wlbragg]

OK, like I said, I'm not opposed to the variant approach, I understand that's what it's there for. I know this affects our ability to change variants on the fly and I think it is a shame to loose the flexibility of being able to change all these features "in sim", all because of a controller binding. I'm not convinced about the reasons we need to do this, just my opinion, but mostly because I still don't understand why we can't add code to either or Nasal/controls.nas and/or

You can configure a joystick axis to affect multiple engines (see controls.nas perEngineSelectedAxisHandler, /controls/engines/throttle-all), so you can have one lever controlling the port side engines, and the other controlling the starboard. Additionally, there is code to select engines for the keyboard throttle.

I am aware though we had to apply a few filters to accommodate this approach, but it was how it had to be done to achieve the outcome. As far as a hack, I guess it's a matter of perspective. Depends on you ultimate goal. As the core code and supports stands now, maybe more of a hack than not, but that's not to say we couldn't take another path to make this a viable option for aircraft. There are now at least a few to several aircraft that offer multiple size engines.
I don't like exiting and waiting just to load options, I think it is not the right approach and we should work to accommodate that. Not the other way around.

But I am not hearing any feedback or support for my position from anyone else on the team @gilbertohasnofb , @legoboyvdlp @dany93 and it appears @onox is OK with removing the added flexibility of how it is now. I guess that's the beauty of the flexibility of open source code and private hangers. I can keep a version however I want it privately. It's not worth a fight in my opinion even though I strongly disagree, at least so far, with the reason why this needs to be done.

As far as the states, that is long overdue, but I'm sure you followed my comments on the mailing list about what really needs to be done to give it justices and not basically be yet another hack!

For now I am going to step back and just do my own thing. Maybe in the future I will come to see the reasoning for this is justified and the right approach for the future as it was for the past.

[dany93]

But I am not hearing any feedback or support for my position from anyone else on the team .... @dany93 ...

This code change is well above my skills.

I'm not rigid on the subject of changing the engine in-sim, however I appreciated this flexibility. But the main flexibility (bushkit, floats, skis) will remain. As the standard gear with the 180hp engine, I guess.
What I will regret is the management, two FDMs to be maintained at each code change.
An advantage is that the engine particularities will be easier to apply (no system files, filters or other code). Probably more clear, easier to understand.

I agree that having to loose this only for one quadrant JS which doesn't work is not very pleasant.

[gilbertohasnofb]

I'm not rigid on the subject of changing the engine in-sim, however I appreciated this flexibility. But the main flexibility (bushkit, floats, skis) will remain. As the standard gear with the 180hp engine, I guess.

This is actually quite a good point to think about. The standard engine on a 172P is the 160hp, and I think we definitely need to fit that in as the default. But that means we will lose the flexibiility with changing to floats or bushkits in-sim, as the 160hp is not powerful enough for those. Making the 180hp engine the default would be wrong IMO, as the 160hp is the most used one in RL by far. The only solution I can think of would be to also remove the flexibility of changing to bushkit and float in-sim and use only the variant approach. This would be a pity.

What I will regret is the management, two FDMs to be maintained at each code change.

Yes, that would be quite annoying to maintain. This single aircraft actually becomes several aircraft. For me this is actually the most annoying point; I could definitely live without the flexibility of changing engines in-sim but maintain two FDMs sounds annoying to say the least, and might lead to bugs in the long run (e.g. only one FDM being altered when fixing a bug, unknowingly causing them to diverge).

[wlbragg]

FYI...

amphibious and float variants use the io360, while the other variants use the io320. It might be worth changing the bush variant to use the io360 as well, if that's a common configuration.

If this PR is going to be executed, there may may be references supporting leaving a variant of all undercarriage change outs for both engines. The variants as they are laid out now grouped the most likely uses attached to the most common engine regimes. This was acceptable because you could still mix and match all features "in sim". There is no reason not to leave a variant for each combination of features. You definitely shouldn't remove the ability to use the bush tires or any other undercarriage from the io320 even if only the "in sim" portion, in my opinion. It would be wrong as I believe it was done and discovered that a larger engine was needed to do it justice!

[stuartbuchanan]

Hi Folks,

Re: FDM maintaining multiple FDMs. There should be minimal difference between the FDMs - just the different engine, prop and a pointmass for the additional engine mass for the io360. (Though I notice that I've left a spurious float chamber in the io360 definition I should remove)

Re: Variants. The PR retains the flexibility to choose whatever set of gear/floats with whatever the chosen engine is. The -set variants really just impact the engine and the starting gear. The user can change the gear in-sim, as before. So to use the 160hp engine with amphibious floats the user would select a 160hp variant in the launcher, and then change to the amphibious floats from the configuration menu in-sim.

@dany93 - this affects all quadrants with multiple throttle/prop/mixture levers. I've verified it on the CH version, and another user hit it on theirs (not sure whether it was CH or Saitek). So it's not just a single piece of hardware.

[dany93]

@stuartbuchanan wrote

Though I notice that I've left a spurious float chamber in the io360 definition I should remove)

Initially, there was a float chamber for each engine.
@wlbragg recently replaced this configuration for a single, common float chamber for both engines.
#1327 (comment)
@wlbragg wrote

As far as the duplicate float chamber. It really is totally unnecessary and a waist of a few lines of code

If I remember well, this float chamber is used, at least in case of negative g's. For 160 and 180 hp engines, of course.

[dany93]

@stuartbuchanan wrote

... this affects all quadrants with multiple throttle/prop/mixture levers.(...)). So it's not just a single piece of hardware.

Obviously this cannot be a hardware issue.
And, as the c172p "active engine" code works with every other joystick and keyboard control, I rather think this is a JS configuration file (binding file) issue.
@wlbragg wrote

I have a throttle quadrant and it works fine. Is that because I mapped it differently than the default?

[stuartbuchanan]

By "So it's not just single piece of hardware" I should have written "So it's not just a single model of hardware".

As I mentioned earlier in the thread, it's a specific issue with quadrants that have multiple throttle controls.

I've done a quick grep, and I think it affects the following standard joystick configuration files that are in fgdata, and which use controls.perEngineSelectedAxisHandler (which is what mine uses):
./ThrustMaster/Warthog/Warthog-Throttle.xml
./Saitek/Aviator.xml
./Saitek/Pro-Flight-Quadrant.xml
./CH/throttle-quadrant.xml

I haven't tested them, but from a code read I'm sure it also affects any throttle control attempting to set /controls/engines/engine[0]/throttle directly. So that also impacts the following
./InterLink/interlink-elite.xml
./Macally/istick-usb.xml
./Microsoft/xbox-360-controller.xml
./LewEngineering/RC-transmitter-hitecLaser4.xml
./GoFlight/tq6-adv.xml
./Logitech/wingman-force.xml
./Logitech/g940.xml

Note that as the default aircraft, it's particularly important that stuff like this works out of the box.

-Stuart

[dany93]

Thanks for this detailed response and research, @stuartbuchanan.

And sorry for my lack of knowledge about this.

[wlbragg]

There are issues with the changes making this only use engine[0]
I haven't finished looking through all of it yet, my time is limited at the moment, but I expect more. In Systems/bushkit.xml lines 61 you just eliminate the 180's extra weight entirely? Is it accounted for elsewhere, maybe in a variant set file?

I have to say what I dislike about this the most is that this change is just decided by one person outside the c172p core team and that this is the needed change to fix this issue. No discussion, no research as to cause and effect, and no attempt to understand what other remedies may be available. This is not how we have managed the changes to this aircraft in this development group. We have had many discussions as to what we want done, how and why.
Do you know why using engine[0} as a binding isn't working. As far as I am aware we map engine[0] to active-engine, as well as engine[1]. So why is engine[0] not working?
The details are what is bothering me. Our "hack" is one new property, internal to the aircraft that controls the FDM and is derived by engine[0] and engine[1].
Why go down this path without fully understanding the cause. this is a massive refactoring, one of which costs added functionality gains, without fully understanding why it is necessary.

I wasn't able to find a way to do so by over-riding the controls.nas module (which has previously caused problems in itself), so spit the FDM file in two based on the engine type (io320 or io360).

Has anyone looked at how we may be able to accommodate these controllers in the aircraft code VS trying to change the controller configurations?

[stuartbuchanan]

Firstly, I'm sorry that this has come across so badly and apologize for any hurt I've caused. It certainly wasn't my intention to cause problems, and this pull request simply represent the result of the analysis I've done and my proposed fix. I think there is a problem here that needs to be fixed, and if there is a better solution than mine then great.

Equally, I'm quite happy to be challenged on my understanding of the problem as well.

Given you're clearly not happy about his, I suggest i go and have another look at how else we might address this.

-Stuart

The 180hp engine extra mass is now part of the FDM file itself (line 195 of c172p_io360.xml)

[dany93]

@wlbragg wrote

you just eliminate the 180's extra weight entirely? Is it accounted for elsewhere, maybe in a variant set file?

It is included in the c172p-io360.xml file, lines 191-200.

        <!-- Extra weight for the aircraft with 180 hp engine, **Basic empty** (= with full oil), pointmass [15]; -->
        <!-- for empty aircraft CG at x = 38.12 ins, from Cessna 172S - N552SP POH (1998) p.6-12, Weight and moment tabulation: 1642 lbs, 62600 lb-ins -->
        <pointmass name="extra weight 180hp">
            <weight unit="LBS"> 175 </weight>
            <location name="POINTMASS" unit="IN">
                <x> 30.29 </x>
                <y>  0 </y>
                <z> 26.6 </z>
            </location>
        </pointmass>

This aside, I would have preferred a more consensual approach too.

[wlbragg]

System/engine.xml

<filter>
     <name>Engine 160 HP Throttle</name>
     <type>gain</type>
     <input>
         <condition>
             <equals>
                 <property>/controls/engines/active-engine</property>
                 <value>0</value>
             </equals>
         </condition>
         <property>/controls/engines/current-engine/throttle</property>
     </input>
     <input>
         <property>/controls/engines/engine[1]/throttle</property>
     </input>
     <output>
         <property>/controls/engines/engine[0]/throttle</property>
     </output>
 </filter>

 <filter>
     <name>Engine 180 HP Throttle</name>
     <type>gain</type>
     <input>
         <condition>
             <equals>
                 <property>/controls/engines/active-engine</property>
                 <value>1</value>
             </equals>
         </condition>
         <property>/controls/engines/current-engine/throttle</property>
     </input>
     <input>
         <value>0.0</value>
     </input>
     <output>
         <property>/controls/engines/engine[1]/throttle</property>
     </output>
 </filter>

[wlbragg]

System/engine.nas

# =============== Variables ================

controls.incThrottle = func {
    var delta = arg[1] * controls.THROTTLE_RATE * getprop("/sim/time/delta-realtime-sec");
    var old_value = getprop("/controls/engines/current-engine/throttle");
    var new_value = std.max(0.0, std.min(old_value + delta, 1.0));
    setprop("/controls/engines/current-engine/throttle", new_value);
};

controls.throttleMouse = func {
    if (!getprop("/devices/status/mice/mouse[0]/button[1]")) {
        return;
    }
    var delta = cmdarg().getNode("offset").getValue() * -4;
    var old_value = getprop("/controls/engines/current-engine/throttle");
    var new_value = std.max(0.0, std.min(old_value + delta, 1.0));
    setprop("/controls/engines/current-engine/throttle", new_value);
};

# 2018.2 introduces new "all" properties for throttle, mixture and prop pitch.
# this is the correct way to interface with the axis based controls - use a listener
# on the *-all property
setlistener("/controls/engines/throttle-all", func{
    var value = (1 - getprop("/controls/engines/throttle-all")) / 2;
    var new_value = std.max(0.0, std.min(value, 1.0));
    setprop("/controls/engines/current-engine/throttle", new_value);
}, 0, 0);

setlistener("/controls/engines/mixture-all", func{
    var value = (1 - getprop("/controls/engines/mixture-all")) / 2;
    var new_value = std.max(0.0, std.min(value, 1.0));
    setprop("/controls/engines/current-engine/mixture", new_value);
}, 0, 0);

# backwards compatibility only - the controls.throttleAxis should not be overridden like this. The joystick binding Throttle (all) has
# been replaced and controls.throttleAxis will not be called from the controls binding - so this is to
# maintain compatibility with existing joystick xml files.
controls.throttleAxis = func {
    var value = (1 - cmdarg().getNode("setting").getValue()) / 2;
    var new_value = std.max(0.0, std.min(value, 1.0));
    setprop("/controls/engines/current-engine/throttle", new_value);
};

controls.adjMixture = func {
    var delta = arg[0] * controls.THROTTLE_RATE * getprop("/sim/time/delta-realtime-sec");
    var old_value = getprop("/controls/engines/current-engine/mixture");
    var new_value = std.max(0.0, std.min(old_value + delta, 1.0));
    setprop("/controls/engines/current-engine/mixture", new_value);
};

# backwards compatibility only - the controls.throttleAxis should not be overridden like this. The joystick binding Throttle (all) has
# been replaced and controls.throttleAxis will not be called from the controls binding - so this is to
# maintain compatibility with existing joystick xml files.
controls.mixtureAxis = func {
    var value = (1 - cmdarg().getNode("setting").getValue()) / 2;
    var new_value = std.max(0.0, std.min(value, 1.0));
    setprop("/controls/engines/current-engine/mixture", new_value);
};

var _axisMode = {
    0: controls.perIndexAxisHandler("/controls/engines/current-engine[",
        "]/throttle"),
    1: controls.perIndexAxisHandler("/controls/engines/current-engine[",
        "]/mixture"),
    2: controls.perIndexAxisHandler("/controls/engines/current-engine[",
        "]/propeller-pitch")
};
controls.perEngineSelectedAxisHandler = func(mode) {
    return _axisMode[mode];
};

[wlbragg]

My throttle quadrant bindings, interesting, it isn't remapped as far as I can tell?

  <axis n="2">
    <desc type="string">Throttle (all)</desc>
    <binding>
      <command type="string">nasal</command>
      <script type="string">controls.throttleAxis();</script>
    </binding>
  </axis>

So what are these other controllers using that is not accounted for in the aircraft code?

[wlbragg]

@stuartbuchanan I never wanted to come off this way. I really am happy with what we have managed to do with this aircraft and I feel like this is a step backwards to undo this flexibility. I understand you probably feel it is a step back to a more standard or accepted norm. That alone makes this problematic for us both. But if there is any other solution that isn't so invasive and accomplishes the task at hand, plus preserves the current functionality, I really would like to find it.

[wlbragg]

Is the issue that these other controllers are calling engine[0]/engine[1] directly vs controls.throttleAxis()?

[stuartbuchanan]

Yes, that's exactly the issue.

(As I think you've spotted, your quadrant binding of controls.throttleAxis() is being remapped by engine.nas (see the middle of the file).)

There is also an override of controls.perEngineSelectedAxisHandle(), in engine.nas - I didn't spot it initially as it's right at the end of the file. However, the Joystick Configuration dialog (and my quadrant) doesn't use it - it sets /controls/engines/engine[]/throttle etc directly. From the comments in joystick.nas, this was an explicit decision so that those axes could be inverted, which perEngineSelectedAxisHandle() doesn't support.

I took another look at modifying the core control bindings. It might be possible, but would require a lot of work on the core side and would still leave some problems:

It would require modifying perEngineSelectedAxisHandler() to support inverted axes and then modify the joystick configuration dialog to use it. I think that would just about be possible, but it would still leave a couple of other issues:

• It doesn't fix the existing joystick configurations that use the properties directly, both those provided in fgdata and any user custom definitions. We might change the former, though it would be a little risky as I don't have the hardware available.
• A change to perEngineSelectedAxisHandle would have knock-on effects to the over-riding that the c172p and other aircraft do. . So creates a compatibility headache in itself where there might have to be an update to all the aircraft that over-ride controls.nas. This is one of the main reasons I'm not keen on aircraft over-riding controls.nas ;).

I've done some further investigation to see if there's another way to address this without the refactoring, and come up with one possibility :

As you've spotted, there only a mapping from /controls/engines/current-engine/... to /controls/engines/engine[0] and /controls/engines/engine[1]. I don't think it would be possible to get a mapping to go the other way because you end up in a loop. It might be possible to create a dummy engine[0] and then use that for the controls mapping to the "real" engine[1] and engine[2]. So that would be a possible solution, and would mean that the normal /controls/engines/engine[0]/throttle etc would work, while retaining the flexibility of in-sim engine changes.

[wlbragg]

From the Aviator.xml controller

<!-- Analog axis 2. Throttle 1 -->
 <axis>
  <name>Left throttle</name>
  <number>
   <unix>2</unix>
   <mac>3</mac>
   <windows>2</windows>
  </number>
  <desc>TM0: throttle, TM1: throttle/propeller 1, TM2: throttle/propeller 3</desc>
  <binding>
   <command>nasal</command>
   <script>
     if (engine_select_mode == 0) {
       controls.throttleAxis();
     } else {
       controls.perEngineSelectedAxisHandler(engine_axis_mode)
           ((engine_select_mode == 1) ? engine[0] : engine[2]);
     }
   </script>
  </binding>
 </axis>

 <!-- Analog axis 4. Throttle 2 -->
 <axis>
  <name>Right throttle</name>
  <number>
   <unix>4</unix>
   <mac>4</mac>
   <windows>4</windows>
  </number>
  <desc>TM0: mixture, TM1: throttle/propeller 2, TM2: throttle/propeller 4</desc>
  <binding>
   <command>nasal</command>
   <script>
     if (engine_select_mode == 0) {
       controls.mixtureAxis();
     } else {
       controls.perEngineSelectedAxisHandler(engine_axis_mode)
           ((engine_select_mode == 1) ? engine[1] : engine[3]);
     }
   </script>
  </binding>
 </axis>

[wlbragg]

@stuartbuchanan why do we use both engine[x] and/or the nasal wrapper controls.xxx in controller configurations? Shouldn't we really only be using the nasal wrapper version?

Where is /controls/engines/engine[x]/throttle actually used in any aircraft FDM? I did a "find in files" on the c172p and the only reference is the above filter mapping current-engine to engine[0]? Yet that is the active property that is controlling the engine?
Is that like a "tied property"?

[wlbragg]

From my understanding, using /controls/engines/engine[x]/throttle in a controller binding eliminates the aircraft code from being able to use that property in any way, that can't be right. If so then the controllers shouldn't be using it as a direct binding.

[wlbragg]

I'm not sure what you mean by "dummy" property, but I can't see a way to use /controls/engines/engine[x]/throttle in aircraft side code for anything so long as a controller is using it as a direct call to the FDM throttle. What that in effect does is tie the controller to the native JSBSim FDM bypassing any attempt to control the throttle in the aircraft code. Maybe I am not seeing this clearly.

[wlbragg]

It might be possible to create a dummy engine[0] and then use that for the controls mapping to the "real" engine[1] and engine[2]. So that would be a possible solution, and would mean that the normal /controls/engines/engine[0]/throttle etc would work, while retaining the flexibility of in-sim engine changes.

Unless you can explain how this works, I see no other solution other than your proposed solution of variants OR not allowing joystick controllers to map directly to ...engine[x]/throttle!

[wlbragg]

Firstly, I'm sorry that this has come across so badly and apologize for any hurt I've caused. It certainly wasn't my intention to cause problems, and this pull request simply represent the result of the analysis I've done and my proposed fix. I think there is a problem here that needs to be fixed, and if there is a better solution than mine then great.

Equally, I'm quite happy to be challenged on my understanding of the problem as well.

Given you're clearly not happy about his, I suggest i go and have another look at how else we might address this.

@stuartbuchanan

First, I apologize, you didn't come off badly, I just needed to understand all the ramifications and options before I could be OK with this. We usually discuss minor stuff and this is major.

Please read my responses above. No, I am really not happy about this change. But if it is insisted that a controller be allowed to use controls/engine/engine[x]/throttle directly, then I agree with you, this is the only way to make this work. I still don't understand how controls/engine/engine[x]/throttle ties directly to the FDM throttle outside our FDM? I see no link or connection in our local FDM code. That puzzles me. I think it is wrong overall because using this property directly from the controller eliminated the aircraft side code ability to filter that property in any way. If controls/engine/engine[x]/throttle was actually being tied to a local fcs function then we could easily filter it and do whatever we wanted to do with it.
Can you or @dany93 explain this to me, as to how this works?

[dany93]

@wlbragg wrote

Can you or @dany93 explain this to me, as to how this works?

Unfortunately, no. I try to follow your discussion but I am discovering these direct and indirect ways of controlling...
And I have a very hard time at trying to understand the controls.nas file.

[stuartbuchanan]

Hi Folks,

Good news!

I've managed to write the XML control logic to map correctly to the JSBSim properties directly.

This means

1. Retain the ability to change engines in-sim.
2. One single FDM definition
3. /controls/engines/engine[0]/throttle can be used for both 160 and 180 engines.
4. No need to over-write controls.nas logic.
5. All the joysticks and quadrants setting /controls/engines/engine[0]/throttle will Just Work.

So I think this achieves both your design goals and the requirement for the standard properties to Just Work.

I need to do some work to replace /controls/engines/current-engine/throttle and /controls/engines/current-engine/mixture with
/controls/engines/engine[0]/throttle and /controls/engines/engine[0]/mixture, but this will be a much less invasive change than my original version.

-Stuart

[stuartbuchanan]

False alarm :(.

What I thought was working for both engines was in fact just working for engine[0]. Which isn't any use. :(

I'll have another look at this with fresh eyes tomorrow.

-Stuart

[wkitty42]

On 9/29/20 6:11 PM, stuartbuchanan wrote: False alarm :(. What I thought was working for both engines was in fact just working for engine[0]. Which isn't any use. :(

is there some way to alias the 160 or the 180 to engine[0] as needed? or the other way around?

[wlbragg]

@stuartbuchanan

There's been a long-standing bug with the c172p that hardware throttle quadrants don't work. The root cause is the model having multiple engine definitions and logic to select an active-engine.

Stuart, is it both engine throttle mappings that don't work on your controller or just the one using engine[1] ?
What I am asking is exactly how you use your multi throttle quadrant?
Do you typically expect to use engine[0] on all aircraft with a single engine and then engine[1] on aircraft with multiple engines (in aircraft that support "twin" engines?
But in the c172p both engines could be considered to be only one engine, either engine using only engine[0].
So is it that the 160 engine works but the 180 doesn't?
or is it...
Neither 160 or 180 work?
If you could continue to call /controls/engines/engine[0]/throttle and /controls/engines/engine[1]/throttle directly, what would you want to be able to use for the 160 and 180 engines, only the engine[0] property to drive either engine, or 0 for the 160 and 1 for the 180?

[wlbragg]

@stuartbuchanan

What I thought was working for both engines was in fact just working for engine[0]. Which isn't any use. :(

That actually might be workable. If you have a direct call to engine[0] working then we may be able to map engine[1] differently by using engine[0]. Can you post the changes you made so I can follow along?

I think I answered my above question about which engine is not working with which mapping. I changed my joystick mapping to /controls/engines/engine[0]/throttle and then also to 1 and neither worked. So I answered that question. But I still would like to know...

If you could continue to call /controls/engines/engine[0]/throttle and /controls/engines/engine[1]/throttle directly, what would you want to be able to use for the 160 and 180 engines, only the engine[0] property to drive either engine, or 0 for the 160 and 1 for the 180?

[stuartbuchanan]

Hi wlbragg,

It's just engine[1].

Yes, for all engines with a single engine, the user will
expect /controls/engines/engine[0]/ controls to work.

So I can make the 160hp engine work, but not the 180hp engine.

The ideal would be that /controls/engines/engine[0]/throttle would work for both the 160 and 180 engines.

Here the autopilot control file I've been trying to get work to map from those properties to the internal JSBSim ones. They work in that they set the properties, but there is a further transformation from the "pos" to the "cmd" properties that currently isn't :(.

control-mapping.zip

I've got one further idea - use autopilot config files to map from /controls/engines/engine[0] to /controls/engines/engine[1] directly.

-Stuart

[wlbragg]

I've got one further idea - use autopilot config files to map from /controls/engines/engine[0] to /controls/engines/engine[1] directly.

That is where I was going if you actually got engine[0] to work directly.

[wlbragg]

@stuartbuchanan
So far I am not even getting the results you got using /fdm/jsbsim/fcs/throttle-pos-norm[0] in Systems/control-mapping.xml.
Are you just including those new filters alone? Or are you also changing the filters in Systems/engine.xml

[stuartbuchanan]

There are changes elsewhere. I've just pushed them here: https://github.com/stuartbuchanan/c172p-detailed/tree/enginefilter

-Stuart

[stuartbuchanan]

Hi Folks,

Having spent some more time on this, I'm at a dead end. I've been unable to get the autopilot filters to work in the way I need them to.

Mapping to /controls/engines/engine[0] to /fdm/jsbsim/fcs/throttle-pos-norm[1] doesn't work, and neither does mapping from /controls/engines/engine[0] to /controls/engines/engine[1]. In both cases the values from the "proper" joystick axis end up over-riding it.

So I think we're back to balancing which is more important: - being able to use the /controls/engines/engine[] properties directly, or being able to change the engine in-sim.

-Stuart

[wlbragg]

@stuartbuchanan you have been extremely patient with me and I truly appreciate it and ask you for just a bit more indulgence.

So I think we're back to balancing which is more important: - being able to use the /controls/engines/engine[] properties directly, or being able to change the engine in-sim.

OR,

Is there a third option (desirable or not) to NOT use /controls/engines/engine[] directly but only use the mappings that the other half of the controllers use, such as controls.throttleAxis()?

I ask becasue I am still unclear of the validity of a controller controlling "THIS" aircraft using /controls/engines/engine[] VS controls.throttleAxis().
Why was controls.throttleAxis() helper implemented?
Don't we have mappings on a "per aircraft basis"?

Just because /controls/engines/engine[] has been used traditionally doesn't necessarily mean it is the best way to do this. Especially if there is the option to simply remap the existing controllers and retain the ability to have this engine flexibility. It that possibility doesn't' exist, then I concede and it would be a different story.

OK, that's my questions as best I can describe them with absolutely no disrespect implied and understanding your experience in all things FlightGear.

Maybe these questions will sum up my hesitation to change this and why I am so stuck on this opinion.

If the c172p had always been configured this way would these joysticks be configured to work with it?
And again, can these controllers be configured to work with it through their mappings? If not can you please explain why?

If they can be configured to work through their mappings, I would take on the responsibility to do the work with your guidance.

[wlbragg]

p.s. I haven't had the chance to look at where you are with getting engine[0] working but not mapping 1 to 0, and will look at it tonight and verify that I come to the same conclusion as you.

[wlbragg]

okroth commented on Aug 10 •

For the 172R exists a Cessna documentation MK172-72-01C that explains how to upgrade from 160HP to 180HP. However, the engine stays the same IOC360L2A, it just gets a new propeller, engine instruments, etc. The higher power rating comes from higher RPM allowed (2700 instead 2500)

Member
dany93 commented on Aug 10

@okroth wrote
However, the engine stays the same IOC360L2A

If I understand well, the IOC360L2A engine is from the IO360 family, already a 180 hp engine.
@okroth
Author
okroth commented on Aug 10

Correct. But rated at 160 hp in the standard configuration with 2500 rpm limitation

@dany93 is this an option?
As stated above, If this is legit, why can't we get rid of the 160 and degrade the 180 to the specs of the 160 instead of two engines for our engine switch?

[stuartbuchanan]

@wlbragg - That third option would be effectively to deprecate using the /controls/engines[]/ properties directly. I think that's quite a major conceptual change that you'd need to propose on the -devel list and get agreement on. Personally, I don't think I would support that change, but I'd be happy to help explain the technical side if you needed assistance so it can be considered. I'm not going to throw my toys out of the pram if there was a consensus supporting it :)

Good question on controls.throttleAxis(). That was introduced to make it easier to allow a single joystick to control any number of engines. So instead of having a joystick axis with separate bindings for each of the engines, you could just have one.

You ask if joysticks could be configured to use the controls.throttleAxis() bindings: Yes, absolutely. I think we covered that above. The challenge is a) testing because we don't have access to the controllers to ensure the change works, b) we still have issues with existing bindings and devices that aren't covered by the bindings in fgdata/Input. Unfortunately the c172p is held to a higher bar because it's the default aircraft. We really need it to work "out of the box" as much as possible.

In terms of having a single engine definition that covers both the 160 and 180hp engines, I think the stumbling block is going to be the propeller definition.

-Stuart

[wlbragg]

@stuartbuchanan

OR
Is there a third option (desirable or not) to NOT use /controls/engines/engine[] directly but only use the mappings that the other half of the controllers use, such as controls.throttleAxis()?

That third option would be effectively to deprecate using the /controls/engines[]/ properties directly. I think that's quite a major conceptual change that you'd need to propose on the -devel list and get agreement on. Personally, I don't think I would support that change, but I'd be happy to help explain the technical side if you needed assistance so it can be considered. I'm not going to throw my toys out of the pram if there was a consensus supporting it :)

Especially if there is the option to simply remap the existing controllers and retain the ability to have this engine flexibility. It that possibility doesn't' exist, then I concede and it would be a different story.

Sorry you misunderstood me, the third option is the same as "remapping". Just don't use /controllers/./../engine[x]/trhrottle directly anymore.
Your not saying if we made this change your controller would no longer work. Because you state this later...

You ask if joysticks could be configured to use the controls.throttleAxis() bindings: Yes, absolutely. I think we covered that above. The challenge is a) testing because we don't have access to the controllers to ensure the change works, b) we still have issues with existing bindings and devices that aren't covered by the bindings in fgdata/Input. Unfortunately the c172p is held to a higher bar because it's the default aircraft. We really need it to work "out of the box" as much as possible.

If so, if that broke your controller, then I don't consider this an option either. I would never push for that.

The challenge is a) testing because we don't have access to the controllers to ensure the change works, b) we still have issues with existing bindings and devices that aren't covered by the bindings in fgdata/Input.

So this is where I am at as a last ditch solution. I feel that if these controllers are such an issue we would have heard about it by now. My guess is they are one offs or the users have already changed their bindings to something that works. So "needing testing" falls in that train of thought as well. If anyone has issues we generally hear about it.

In terms of having a single engine definition that covers both the 160 and 180hp engines, I think the stumbling block is going to be the propeller definition.

We can use multiple pitch on the prop. Or are you saying we need a completely different prop and we can't change that on the fly?

[dany93]

@wlbragg wrote

As stated above, If this is legit, why can't we get rid of the 160 and degrade the 180 to the specs of the 160 instead of two engines for our engine switch?

From the reality point of view, I don't understand the interest for this 'upgrade'. Can it be for a few people who wished to postpone their investment? Otherwise, why investing in an injection 180 hp engine and bridle it at 160 hp? Knowing that you will have to change the propeller + some instruments later to use it at 180 hp...
From the technical point of view, I see zero interest. (except after more thinking, less wearing of the engine)

The degradation proceeding (180 --> 160 hp) is obviously out of interest too.

From the simulation point of view:

• the MK172-72-01C engine is an injection one. Ours are carburetor ones (differently from that the wrong file names may make to believe).
• Notice we will still have to switch for a second propeller (which diameter?)
  Also, @stuartbuchanan' s remark:

In terms of having a single engine definition that covers both the 160 and 180hp engines, I think the stumbling block is going to be the propeller definition.

• In JSBSim, the engine is basically a constant-torque one vs RPM [1]. Which would give 180 x (2500/2700) = 167 hp at 2500 RPM. Not 260.
  Which means that we will have to cheat with (I don't know which) parameters to bridle it for 2500 RPM max and 160 hp at full throttle. How? By starving it? And cheating is cheating, with possible future unexpected effects. With the risk that someone later says "fuel consumption does not fit the reality, or EGT vs mixture, or...". We will have to replace the current 160hp engine for an articially hacked one, while both current engines are based on the jSBSim core code, which has hopefully been coded by people who knew what they did.

[1] Please fix me if I'm wrong

[dany93]

@wlbragg wrote

We can use multiple pitch on the prop. Or are you saying we need a completely different prop and we can't change that on the fly?

At least, probably a different prop diameter.
I think @stuartbuchanan means that the propeller is defined at the same time, same block in the FDM as the engine.

    <propulsion>
        <engine file="eng_io320">
            <thruster file="prop_75in2f">

[wlbragg]

@dany93
Thanks for clarifying. My confusion is...

If I understand well, the IOC360L2A engine is from the IO360 family, already a 180 hp engine.

then this

Correct. But rated at 160 hp in the standard configuration with 2500 rpm limitation

This sounds to me like the engine is the same engine configured differently. So on the surface is seems we are already "cheating" in the way we did this? Unless we did exactly that and used the same engine parameters less available RPM? But to do that we needed two different engine definitions?
I need to take my time and really decipher what you said and see if it makes more sense.

[dany93]

In JSBSim, one engine giving 180 hp at 2700 RPM and one giving 160 hp at 2500 RPM are two different engines.
And we currently have two engines (which is cheating, but with no consequences on engine functioning) but no cheating in each engine configuration.

[stuartbuchanan]

@wlbragg - I'm afraid I still don't understand what your option 3 is. Unless it's to change the existing joystick bindings in fgdata/Input, and deprecate using the properties directly, which as I said above is something you'd really need to get buy-in from the -devel list. Or am I missing something?

So this is where I am at as a last ditch solution. I feel that if these controllers are such an issue we would have heard about it by now. My guess is they are one offs or the users have already changed their bindings to something that works. So "needing testing" falls in that train of thought as well. If anyone has issues we generally hear about it.

Well, I'm telling you about it now. :)

I don't think we see half the issues that end-users encounter. This isn't unique to this particular problem, but a general issue where I suspect a lot of new users hit problems and immediately give up because at that point they haven't got anything invested in the software. They shrug their shoulders and move on.

I've reached the limits of the time I can spend on this. I've now spent significantly more time code-reading, explaining what is going on in the code and investigating alternatives than I did on the original work. BTW - I'm happy to have done so, as I learnt some stuff in the process. However I think we're now running round in circles.

-Stuart

[wlbragg]

I'm afraid I still don't understand what your option 3 is. Unless it's to change the existing joystick bindings in fgdata/Input, and deprecate using the properties directly, which as I said above is something you'd really need to get buy-in from the -devel list. Or am I missing something?

Yes and no, yes, don't use it directly for the c172p. Do what ever you want any other aircraft. Especially if there is a "per aircraft binding capability". If it takes a dev list consensus so be it.

I suspect a lot of new users hit problems and immediately give up because at that point they haven't got anything invested in the software. They shrug their shoulders and move on.

The term "Suspect" doesn't hold much weight in my opinion. I suspect it is because most user simply change their bindings.

I've reached the limits of the time I can spend on this.

Me too right now at this moment in time.

Bottom line with me is...
Why have itermediate, per aircraft binding capabilities if we are not allowed to leverage them?
Why should we make such a major code revision in aircraft code VS a simple controller binding change?
Why loose flexibility plus current and potentially future options because we don't want to change from a hard coded property to an intermediate layer?

No matter how hard I try to be ok with this, so long as the bindings can be changed and the problem is solved, I can't.

So I guess we are at an impasse and I am not interested in making this change if it can be handled with a binding.

[wlbragg]

I want to make it clear, that if it can't be fixed with a binding change then I would support this.
Thank you for making such an effort to solve this in a manner to make us both satisfied.

[wkitty42]

i don't know if i'm going to ask this properly but i'm going to try...

why can't we have similar to what we currently have with two engine definitions... call them engine160 and engine180... they're in their own separate property trees... when we select the 180, map/alias these onto engine[0] and go... if we select the 160, map/alias those onto engine[0]...

or is the problem that we cannot change the FDM at runtime so it has to have both engines in it all the time? could this be handled by adjusting the craft init code in core so we can get the FDM switch in place before JSBSim eats it for operation? kinda like when we switch airports or scenery settings and the sim basically restarts everything... just do the same for an engine change like this...

[wlbragg]

@wkitty42 what your asking is on an order of magnitude a whole bunch more than either fix Stuart and I are talking about and probably is a non starter. But thanks for the suggestion. With no other fix other than Stuart's original idea or simply changing a controller binding (doesn't matter to me how many hardware bindings need changed), I just can't make myself be OK with changing the aircraft to support a binding as I can with changing a binding to support an aircraft. I wish I could and that it didn't matter to me, but it does because it takes away a flexibility in programming that I don't want to loose.
I understand if that is how this has to go down, but I don't have to help facilitate this change and I will, at least for now, keep this version in a private hanger along with the J3Cub/PA18 which incorporates the same internal mechanics only times 3 engines.
So we truly are at an impasse and I feel terrible about it because I know @stuartbuchanan is just trying to make this work as best he thinks it should. We simply don't agree on how.

[dany93]

@stuartbuchanan wrote

...I suspect a lot of new users hit problems and immediately give up because at that point they haven't got anything invested in the software. They shrug their shoulders and move on.

@wlbragg wrote

The term "Suspect" doesn't hold much weight in my opinion. I suspect it is because most user simply change their bindings.

Maybe posting a question in the forum, asking if other people had the issue? And, if yes, which hardware and if / how they solved it?