-
Notifications
You must be signed in to change notification settings - Fork 36
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
Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero #302
base: master
Are you sure you want to change the base?
Log polar error code, Log polar parsing warnings + polar parsing logic when value is zero #302
Conversation
src/Polar.cpp
Outdated
@@ -304,6 +304,7 @@ bool Polar::Open(const wxString &filename, wxString &message) | |||
"To specify interpolated, leave blank values.\n" | |||
"To specify course as 'invalid' use 0.0 rather than 0\n")); | |||
warn_zeros = true; | |||
s = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was it intentional that s was set to NaN
for the first element?
it was intentional... 0 means the boat is not moving. NaN means we do not know the boat speed.. but maybe it is still broken? |
Thanks, the history is helpful. I will check again and get back. |
311a35e
to
2df2559
Compare
Based on this code: PARSE_WARNING(_("Warning: 0 values found in polar.\n"
"These measurements will be interpolated.\n"
"To specify interpolated, leave blank values.\n"
"To specify course as 'invalid' use 0.0 rather than 0\n")); and:
and:
There are four possible values in a polar:
Values are interpolated only if either:
In other cases, the polar value is not interpolated and remains Once the polar values have been loaded and the plugin calculates a route, the value I'm wondering if this logic could be simplified.
For case 3.i, the code hits this path:
It the logic remains as is, then perhaps the values in |
Problem statement
Polar::wind_speed
is set toNaN
. It seems it should be set to zero.Polar::wind_speed
is set toNaN
, thePolar::Speed
function may returnNan
, and a message is printed to stdout. It should probably have been logged to opencpn.log, which makes it easier to troubleshoot.Example print statements to
stdout
when polar contains NaN:Changes in this PR
opencpn.log
. This is useful for troubleshooting route calculation issues. For example:When a polar contains the 0 value, initialize the first element of the
Polar::wind_speed
array with the0
value instead ofNaN
.Polar::wind_speed
array toNaN
was intentional?? See analysis below.When route calculation fails, use
wxLogMessage
instead ofprintf
to report the problem. Improve the message to make it easier to troubleshoot the issue.With this PR, the print statements are now written to
opencpn.log
:Analysis
When a polar is loaded, some warnings are generated with
PARSE_WARNING
, but these warnings are not necessarily displayed anywhere. The warnings are not logged in opencpn.log either. This can make route failure harder to understand.In particular, when a polar file contains a 0 value,
Polar::Open
adds theNaN
value to the first entry in thePolar::wind_speeds
array. For example, given the following polar:The
Polar::wind_speeds
array is populated as[NaN, 5.4, 5.6, 6, 6.1, 6, 5.6, 5.0, 4.3]
Relevant code:
weather_routing_pi/src/Polar.cpp
Lines 299 to 315 in 97f2a08
The
NaN
value in thewind_speeds
array ends up causingPolar::Speed()
to returnVB=NaN
atweather_routing_pi/src/Polar.cpp
Line 537 in 97f2a08