Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation of aircraft XML definition file #1038

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bcoconni
Copy link
Member

@bcoconni bcoconni commented Feb 11, 2024

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 XML validation #155 (comment) 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:

  • It makes all the XML files consistent.
    • For example, one can use tags that are unknown to JSBSim. Such tags will be plainly ignored by JSBSim which is OK if is the user intent but is wrong if the tag has been misspelled.
    • The sections will be at the same place from one aircraft definition to the other. Which, at least within JSBSim repository, can help one to review and understand how the FDM are designed.

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

<aerosurface_scale name="Flap Position Normalizer">
<input>fcs/flap-pos-deg</input>
<domain>
<min>0</min> <!-- Flaps actual minimum position -->
<max>30</max> <!-- Flaps actual maximum position -->
</domain>
<range>
<min>0</min> <!-- Flaps normalized minimum position -->
<max>1</max> <!-- Flaps normalized maximum position -->
</range>
<output>fcs/flap-pos-norm</output>
</aerosurface_scale>
</kinematic>

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
Copy link
Member

@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
Copy link
Member Author

bcoconni commented Mar 1, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants