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

Avoid misleading messages about properties being already defined #864

Merged
merged 2 commits into from
Mar 24, 2023

Conversation

bcoconni
Copy link
Member

JSBSim issues warning messages "Property x/y/z is already defined" when encountering a property definition in an definition XML file while the property exists.

This warning message puzzles users as it is not obvious to understand what this message exactly means. This has been reported several times:

The code that issues this warning message is located in

if (PM->HasNode(interface_property_string)) {
if (override) {
node = PM->GetNode(interface_property_string);
if (FGJSBBase::debug_lvl > 0) {
if (interface_prop_initial_value.find(node) == interface_prop_initial_value.end()) {
cout << property_element->ReadFrom()
<< " The following property will be overridden but it has not been" << endl
<< " defined in the current model '" << el->GetName() << "'" << endl;
}
cout << " " << "Overriding value for property " << interface_property_string << endl
<< " (old value: " << node->getDoubleValue() << " new value: " << value << ")"
<< endl << endl;
}
node->setDoubleValue(value);
}
else {
cerr << property_element->ReadFrom()
<< " Property " << interface_property_string
<< " is already defined." << endl;
property_element = el->FindNextElement("property");
continue;
}

It is a feature that allows to specify the initial value of a property:

<property value="1.0"> system/variable </property>

This statement is very similar to a variable declaration in C++ such as double x=1.0;.

When JSBSim meets this statement, it creates the property system/variable and sets its value to 1.0.

However if the property system/variable already exists then JSBSim considers that the property has already been defined (and possibly initialized) elsewhere. So JSBSim ignores this XML statement and issues a warning message Property system/variable is already defined to avoid overriding the property initialization (possibly with an inconsistent value).

Alternatively, JSBSim allows declaring a property without the value attribute:

<property> system/variable </property>

This variant of the statement is similar to a C++ variable declaration without an initialization such as double x;. It is meant to tell JSBSim to create a property if it does not already exist. This is useful because some XML statements might need a property to exist at the moment of their creation while the property is actually defined at some other place in the XML aircraft definition files. Once again, this is very similar to the declaration of a C++ global variable or function.

If the property system/variable already exists then JSBSim issues the warning message Property system/variable is already defined.

The point of this PR is that it is useless to issue this warning message when the XML statement does not specify the value. As nothing wrong can derive from ignoring the XML statement <property> system/variable </property> when the property system/variable already exists.

This would save a number of warning messages that are issued when loading some FDM, including some of the example models that are distributed with JSBSim.


I also took opportunity of this PR to rename the variable override to override_props since, as @ermarch mentioned in the discussion #854, override is a C++ identifier since C++11. Strictly speaking override is not a reserved keyword and compilers do not issue warning messages when using override as a variable name (neither gcc, nor MSVC or clang). However it is most likely not good practice to do so as it might bring confusion. What about

void override(bool override) override;

(which is legal in C++) 😉 ?

@bcoconni bcoconni linked an issue Mar 24, 2023 that may be closed by this pull request
@seanmcleod
Copy link
Member

Looks good.

@codecov
Copy link

codecov bot commented Mar 24, 2023

Codecov Report

Merging #864 (2d8b070) into master (398c07c) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #864      +/-   ##
==========================================
- Coverage   23.24%   23.24%   -0.01%     
==========================================
  Files         167      167              
  Lines       19603    19607       +4     
==========================================
  Hits         4557     4557              
- Misses      15046    15050       +4     
Impacted Files Coverage Δ
src/input_output/FGPropertyReader.cpp 5.26% <0.00%> (-0.40%) ⬇️
src/input_output/FGPropertyReader.h 0.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@bcoconni bcoconni merged commit 8748f38 into JSBSim-Team:master Mar 24, 2023
@bcoconni bcoconni deleted the prop_error_messages branch March 24, 2023 22:01
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.

FlightGear: useless warnings
2 participants