JSBSim-Team / jsbsim

An open source flight dynamics & control software library
GNU Lesser General Public License v2.1
1.22k stars 420 forks source link

Validation of aircraft XML definition file #1038

Open bcoconni opened 2 months ago

bcoconni commented 2 months ago

This PR contributes to addressing the issue #155. It updates the XML validation document JSBSim.xsd to (nearly ?) match the current state of aircraft definition files format. Some code has been moved from JSBSimScript.xsd and JSBSimSystems.xsd to JSBSimCommon.xsd to reduce redundancy.

Disclaimer

That's a very long introductory comment for a PR and it will most likely be affected by the bike-shed effect a.k.a. law of triviality. I have mostly created it to initiate the discussion. There are a number of choices I made that certainly need to be reviewed and in all likeliness, I'll have to break this PR down into several PR's to make it easier to review.

Consider this PR as a basis for discussion. It is not meant to be merged as is. I have chosen the form of a PR because the changed files show what the result would look like.

Limitations

One of the main limitation to the XSD format is that it does not allow configurations where you could have as many sections as you want in whichever order that suits you. Actually one have to pick one option between the following ones:

  1. Using <xs:sequence>: allows as many instances of each section as the user deems so but the order is imposed.
  2. Using <xs:all>: allows at most one instance of each section without restriction on the order in which they are listed.

Limitations of <xs:sequence>

For example, using <xs:sequence> if the following file validates:

<fdm_config>
  <aerodynamics/>
  <output/>
  <output/>
</fdm_config>

then the same file with a different order of sections is rejected:

<fdm_config>
  <output/> <!-- Error: was expecting 'aerodynamics' -->
  <aerodynamics/>
  <output/>
</fdm_config>

Limitations of <xs:all>

Using <xs:all> validates the following files:

<fdm_config>
  <aerodynamics/>
  <output/>
</fdm_config>
<fdm_config>
  <output/>
  <aerodynamics/>
</fdm_config>

but rejects this file as there are 2 instances of <output/>:

<fdm_config>
  <aerodynamics/>
  <output/>
  <output/>
</fdm_config>

Mitigation for JSBSim

In JSBSim there are a number of models that allows multiple <output> elements or so. For such models the only available option is <xs:sequence>. The consequence is that for XML validation to succeed, an aircraft definition file must meet the following order:

<fdm_config>
    <fileheader/>         <!-- exactly 1 occurrence -->
    <planet/>             <!-- at most 1 occurrence -->
    <metrics/>            <!-- exactly 1 occurrence -->
    <mass_balance/>       <!-- exactly 1 occurrence -->
    <ground_reactions/>   <!-- at most 1 occurrence -->
    <external_reactions/> <!-- at most 1 occurrence -->
    <buoyant_forces/>     <!-- at most 1 occurrence -->
    <propulsion/>         <!-- at most 1 occurrence -->
    <system/>             <!-- any number, or <autopilot/> or <flight_control/> -->
    <aerodynamics/>       <!-- at most 1 occurrence -->
    <input/>              <!-- any number of occurrences -->
    <output/>             <!-- any number of occurrences -->
</fdm_config>

Notes

  1. This order is completely arbitrary and is open for discussion. It was chosen based on legacy and mostly empirically while trying to make all aircraft definition files validate.
  2. @andgi made a comment https://github.com/JSBSim-Team/jsbsim/issues/155#issuecomment-562511315 that states that the order in which the sections above are supplied matter. I'd appreciate to have some examples so that we can discuss how to address this. At least in theory the C++ code is order independent: the order in which XML files are processed by JSBSim is imposed by the C++ code, not by the order in which the items are written in the XML file.
  3. Validating file will remain optional so if some users do not want to meet the above order requirement then JSBSim will still be able to run their models. However they will lack the benefit of checking their models for typos, unsupported features, etc.

Results/Achievements

There are good reasons to validate XML files:

Modifications

Example

There was a great deal of variation (and creativity !) in the way in which <fileheader> was filled. The proposed schema is

<fileheader>
    <author/>           <!-- any number, or <email/> or <organization/> -->
    <license/>          <!-- at most 1 occurence -->
    <sensitivity/>      <!-- at most 1 occurrence -->
    <filecreationdate/> <!-- at most 1 occurrence -->
    <version/>          <!-- at most 1 occurrence -->
    <copyright/>        <!-- at most 1 occurrence -->
    <description/>      <!-- at most 1 occurrence -->
    <limitation/>       <!-- any number, or <reference/> -->
</fileheader>

Note XSD do not allow spaces around dates so you will find a number of changes such as:

- <filecreationdate> 2024-02-11 </filecreationdate>
+ <filecreationdate>2024-02-11</filecreationdate>

There are a lot of changes of that kind, I will not review them in details here but I'd be happy to discuss them below.

Errors found

Extra name="POINTMASS"

There is a lot of occurrences of <location name="POINTMASS"/> in <pointmass/> items. The attribute name in <location> is not used by JSBSim so I removed it.

Extra <product> around <table>

There is a lot of occurrences of the following sequence of tags:

<product>
 <table>
  ...
 </table>
</product>

Here the <product> item is useless as there is only one item provided (and <product> needs at least 2 to compute a result). In such a case, JSBSim issues a warning and ignores <product> but it is better if the extra <product> would be removed which I did.

Ignored <aerosurface_scale>

In the aircraft B747 there is an <aerosurface_scale> that is included in a <kinematic> element. In such a case, JSBSim silently ignores <aerosurface_scale>. This was most likely a copy-paste error that can easily be detected by XSD schemas https://github.com/JSBSim-Team/jsbsim/blob/92daf8953c32a0a602ab0f48240a07c4a04aadb9/aircraft/B747/B747.xml#L358-L371

Potato or Potato ? i.e. <description> or <documentation> ?

There are a whole lot of occurrences of the <documentation> tag which is illegal per the legacy JSBSim.xsd. As their name implies, these tags are for documentation and are plainly ignored by JSBSim. In this PR, I've followed the legacy and replaced all occurrences of <documentation> by <description>. Retrospectively I'd allow both so that there are less file modifications.

I have also replaced all occurrences of <description> under the <tank> element by XML comments <!-- --> (see the aircraft B17). There again, we could allow the occurrence of a <description> tag for <tanks> and that would reduce the size of the PR.

seanmcleod commented 2 months ago

@bcoconni in terms of the order of elements within <fdm_config> I'm assuming you didn't go through all the aircraft in the repo and change the order manually? I'm assuming you wrote a script to do that? Would be useful to provide such a script to users so that they can use it to re-order their aircraft models.

bcoconni commented 2 months ago

I'm assuming you didn't go through all the aircraft in the repo and change the order manually?

No I did change the order manually. It should indeed be feasible to write a script but I was afraid that it would change the tabulation layout of the files which would make the diff hardly legible.