Add FGAuxiliary setters for host-owned propagation - #1482
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1482 +/- ##
=======================================
Coverage 25.10% 25.10%
=======================================
Files 171 171
Lines 18843 18843
=======================================
Hits 4731 4731
Misses 14112 14112 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Adds Setadot, Setbdot, SetNx, SetNy and SetNz to FGAuxiliary so a host that owns the state integration can inject the alpha/beta rates and body-axis load factors that Auxiliary would otherwise derive from the propagation it is no longer running. The host-owned domain is propagation together with ground reactions. The two are tightly coupled through the 6DoF integration and are externalised as one unit, so when a host takes them over these are among the few derived quantities Auxiliary cannot recompute and must be supplied. Together with the per-model enable property (JSBSim-Team#1481), this is the main JSBSim-core change needed to host the engine inside an externally hosted simulation such as DCS World. The rest reuses existing mechanisms: atmosphere override, FGWinds, and the existing velocity setters. A large capability for a small, additive surface. It follows the state-propagation review that Agostino De Marco raised in JSBSim-Team#1390 and JSBSim-Team#1391.
89c914c to
21c95aa
Compare
|
Overall, it looks good to me, thanks @Zaretto for this solution. |
|
Will take a look on the weekend at this PR and #1481 to refresh my memory of the details. |
|
@Zaretto how come there are so few new Looking at my comment - #1390 (comment) in the previous discussion I noted 12 new |
|
There again I agree with @seanmcleod70 and in addition to that, this looks to me like a quick and dirty implementation to solve the particular issues that the DCS interface is facing. The problem is that in most cases these methods will be no-op's because the content passed to the setters will be overwritten by the method My point here is not to reject the PR upfront but to tell that:
Also why are the setters not used to allow modifying their related properties ? jsbsim/src/models/FGAuxiliary.cpp Lines 438 to 440 in f46a071 jsbsim/src/models/FGAuxiliary.cpp Lines 454 to 455 in f46a071 |
|
@seanmcleod70 With only propagate and ground reactions disabled, Auxiliary still runs, so the only things that @bcoconni I think making these five properties that are already bound read/write is a much better solution than adding methods, and one that will permit this all to be done from the XML. This is not designed as a Q&D way to integrate DCS as I've already done that, as discussed on #1390. This is a real attempt to bring extra functionality into JSBSim that will have more widespread use and increase the flexibility of what JSBSim can be used for. I've already seen one real world case where JSBSim couldn't be used without patches Merging #1481 into this is my preferred approach but I will take guidance. |
Yep, I relooked at the code in that context, i.e. jsbsim/src/models/FGAuxiliary.cpp Lines 176 to 177 in d371f38 I must be missing something? |
|
Yes I agree with @seanmcleod70, if
Yes, I like this idea with one caveat: the properties are currently read-only (they only have a getter) so you will need to re-wire them on the fly to make them read/write as soon as the model is disabled. |
|
Since I first had the idea of using JSBSim inside DCS it's been quite a journey, and the last few months have achieved real progress because now I've got acEFM working with zero changes to JSBSim. Credit for this goes to @seanmcleod70 and @bcoconni for the thorough review of my pull requests that has consistently guided what I've been doing into the right places in the codebase. In the end all that was needed was simply to set the integrators in |
|
@Zaretto great to hear it's as simple as disabling the integrators in JSBSim. Just trying to understand the comment about // Alpha/beta rates - Auxiliary::Run() zeros adot/bdot then recomputes
// from in.vUVWdot (which is stale in DCS mode). We compute from
// frame-to-frame differencing and apply AFTER Run() in update().
if (init_body) {
double factor = 1.0 / dT;
pending_adot = (alpha_rads - last_alpha_rads) * factor;
pending_bdot = (beta_rads - last_beta_rads) * factor;
}
....
....
// DCS-provided Vt for debug logging (Auxiliary will compute its own from vAeroUVW)
double Vt_ms = Magnitude(vx - wind_vx, vy - wind_vy, vz - wind_vz); |
|
Apologies - I didn't expect you to review that before I'd finished it. This has now gone and a general tidy up of the interface is completed; if you get a moment to have a quick look I'd appreciate your input. |
Integration is suppressed by setting the Propagate integrators all to eNone at init. We then inject the state from set_current_state_body_axis which will no longer be overwritten by Propagate. This removes the need for new setters (JSBSim-Team/jsbsim#1482) and the need to disable modules (JSBSim-Team/jsbsim#1481) and also . GroundReactions now runs but we keep it effectively disabled by never extending the gear and by having no contact points. There is still an argument for keeping the ability to disable modules but we will wait and see what the JSBSim-Team decides. EarthPositionAngle is forced to zero every frame because the DCS World's world is static and does not rotate and not doing this would probably cause longitude to drift west. In set_current_state_body_axis the attitude is now set before the body velocity. SetUVW converts body velocity to inertial using the transforms in force and then Propagate::Run's CalculateUVW inverts that using the transforms. Setting the attitude first makes both directions use the same frame. Because this now looks like finished, validated code I have taken the leap to strip all the commented-out FlightGear interface code carried in JSBSim_interface since the port.
|
No problem, I was just quite keen to refresh my memory of the integration of JSBSim as an external flight model for DCS, so went and took a quick look, and given this particular PR with regards to I've taken a new look after your tidy up, in particular glancing through these 2 files. https://github.com/Zaretto/acEFM/blob/master/flyt-EFM-dcsJSBSim/JSBSim_interface.cpp Given your comment about simply needing to disable the integrators in JSBSim, i.e. // DCS owns the state, so set all four Propagate integrators to eNone which
// disables integration and permits injection of the state injected in
// set_current_state_body_axis
fgSetDouble("/fdm/jsbsim/simulation/integrator/rate/rotational", 0);
fgSetDouble("/fdm/jsbsim/simulation/integrator/rate/translational", 0);
fgSetDouble("/fdm/jsbsim/simulation/integrator/position/rotational", 0);
fgSetDouble("/fdm/jsbsim/simulation/integrator/position/translational", 0);I guess my initial thoughts were that the breakdown between DCS and JSBSim was along the lines in the following diagram, with JSBSim's responsibilities in the grey block, and with DCS taking care of the blocks I've circled in pale blue. 
However, looking at the code, it isn't the case that DCS simply takes Rather, you supply DCS with the total force and total moment (excluding gear) from JSBSim and then DCS uses that, with mass information etc. to run the 6-DOF dynamics, and integrates the result that you then feed back into JSBSim. In other words, the blocks I've highlighted in green are also DCS blocks, including the atmosphere. void ed_fm_add_local_force(double &x, double &y, double &z, double &pos_x, double &pos_y, double &pos_z)
{
FGJSBsim *model = get_model();
x = model->fgGetDouble("/fdm/jsbsim/forces/fbx-total-lbs") * LBS_TO_N;
y = -model->fgGetDouble("/fdm/jsbsim/forces/fbz-total-lbs") * LBS_TO_N;
z = model->fgGetDouble("/fdm/jsbsim/forces/fby-total-lbs") * LBS_TO_N;
pos_x = center_of_mass.x;
pos_y = center_of_mass.y;
pos_z = center_of_mass.z;
}void ed_fm_add_local_moment(double &x, double &y, double &z)
{
FGJSBsim *model = get_model();
// jsb
// x + back
// y + right
// z + up
// dcs
// x + forward
// y + up
// z + right
x = model->fgGetDouble("/fdm/jsbsim/moments/l-total-lbsft") * LBSFT_TO_NM;
y = -model->fgGetDouble("/fdm/jsbsim/moments/n-total-lbsft") * LBSFT_TO_NM;
z = model->fgGetDouble("/fdm/jsbsim/moments/m-total-lbsft") * LBSFT_TO_NM;
}You then feed void FGJSBsim::set_current_state_body_axis(
double ax, double ay, double az, // linear acceleration component in body coordinate system
double vx, double vy, double vz, // linear velocity component in body coordinate system
double wind_vx, double wind_vy, double wind_vz, // wind linear velocity component in body coordinate system
double omegadotx, double omegadoty, double omegadotz, // angular accelearation components in body coordinate system
double omegax, double omegay, double omegaz, // angular velocity components in body coordinate system
double yaw, double pitch, double roll, // radians
double alpha_rads, // AoA radians
double beta_rads, //AoS radians
double dT,
double ro_kgm3
)void FGJSBsim::set_altitude(double h_ft)
{
Propagate->SetAltitudeASL(h_ft);
}Maybe we should move this discussion to a Github JSBSim discussion regarding general questions and answers about using JSBSim as an external flight model for DCS so that it's easier for others to find as opposed to being buried within the comment section of this PR? |
@seanmcleod70 agreed |
|
Okay I've created a new discussion item for this at - #1488 |
|
So I guess this PR can now be closed? |
4 participants
Together with the per-model enable property (#1481), these five FGAuxiliary setters are the main JSBSim-core changes that permit running the engine inside an externally hosted simulation such as DCS World.
Adds to FGAuxiliary:
When a host owns the state integration it injects the derived quantities Auxiliary would otherwise compute from the propagation it no longer runs. The host-owned domain is propagation together with ground reactions, tightly coupled through the 6DoF integration and externalised as one unit. These rates and load factors are among the few quantities Auxiliary cannot recompute once that integration is external, so the host supplies them.
Follows the state-propagation review @agodemar raised in #1390 and #1391.