Driver/LX: Show flights with invalid date

[lordfolken]

No description provided.

[MaxKellermann]

Did you consult with the vendor about this?

Passing around illegal date values can result in all sorts of follow-up bugs (like assertion failures), so I'd rather not do this, or only do this after changing the definition of a field to allow illegal values, and then ensuring all code paths allow it.

[lordfolken]

I understand the concern with the invalid date.

I did consult with lxnav:
"The problem is in file creation as the unit didn't get a valid date. V80 does not have its own RTC (GPS), so the date must be received from external GPS."
It seems that the file is created despite the GPS not giving a valid date. (Flarm is chained and powered through the vario's GPS port.).

Would a check that would specifically check for 1980-00-00 be acceptable?

[MaxKellermann]

Unfortunately, LXNAV's explanation doesn't make them sound competent. How can you start recording a flight if you don't have a GPS fix yet? (a GPS fix always comes with a date.)
And if you later know the date, and the correct date is recorded in the file, why does the device still pretend it was 1980?

Would a check that would specifically check for 1980-00-00 be acceptable?

And then what?

[lordfolken]

Unfortunately, LXNAV's explanation doesn't make them sound competent. How can you start recording a flight if you don't have a GPS fix yet? (a GPS fix always comes with a date.) And if you later know the date, and the correct date is recorded in the file, why does the device still pretend it was 1980?

The recording happens when a GPS TIme is established. The (empty) File is created at startup. LOGBOOK returns the igc's file timestamp, not the flight date that is contained in the IGC record.

Would a check that would specifically check for 1980-00-00 be acceptable?

And then what?

Right now if date.isPlausible is false XCSoar simply times out when enumerating the flight log list. You can't download any flight regardless whether it has an invalid file date or not, as the list never appears when one record has the invalid date.

We can either remove the isPlausible check, hence not checking the date for validity at all, or accept the 1980 value in addition to all valid date values.

Or we could add error handling to the isPlausible check and replace the string with "Invalid date" and display it to the user.

[MaxKellermann]

Timeout shouldn't happen. But fixing a timeout by accepting bogus values sounds suspicious. What is the real reason for the timeout?

[lordfolken]

Timeout shouldn't happen. But fixing a timeout by accepting bogus values sounds suspicious. What is the real reason for the timeout?

In short: The operation expects a true from the ReadDate function here:

XCSoar/src/Device/Driver/LX/NanoLogger.cpp

Lines 142 to 154 in 68919b8

	static bool
	ParseLogbookContent(const char *_line, RecordedFlightInfo &info)
	{
	NMEAInputLine line(_line);
	line.Skip();
	unsigned n;
	return line.ReadChecked(n) &&
	ReadFilename(line, info) > 0 &&
	ReadDate(line, info.date) &&
	ReadTime(line, info.start_time) &&
	ReadTime(line, info.end_time);
	}

There is no error handling for the function ParseLogbookContent:

XCSoar/src/Device/Driver/LX/NanoLogger.cpp

Lines 157 to 168 in 68919b8

	ReadLogbookContent(PortNMEAReader &reader, RecordedFlightInfo &info,
	TimeoutClock timeout)
	{
	while (true) {
	const char *line = ReadLogbookLine(reader, timeout);
	if (line == nullptr)
	return false;
	if (ParseLogbookContent(line, info))
	return true;
	}
	}

So bascially ReadLogBookLine is reading the line, discards the lines that are not LOGBOOK related, but ParseLogbookContent returns only valid lines. If the line does not pass the check, this function isn't returning true, and we stay in the whlie loop until timeout.

• There would be the option to add a break from the loop if ParseLogbookContent fails.
  That way the flights which are invalid would not be listed and the operation would continue.
  (I will experiment and amend the PR)

• Then there is the question whether we should fail on an invalid date. In the end what we are doing is not listing the flight
  and preventing the user from downloading the flight. As far as I can see the date is only used for display purposes. But i
  do see the danger from a code robustness point of view.

In the meantime, I have received a beta firmware from Uros @ lxnav. This should resolve the original problem, and the file should only be created once a GPS TIME is valid. He however states that this only fixes the problem for future flights. So the original Problem is still there.

[hjr]

I can understand the wish to access also the igc files with "missing" date, especially when it is an lxnav issue (at least a past one). Bypassing the date check might be risky, yes. I would brute force change the known broken date 0.0.1980 with a known broken one, that passes the date check (e.g. 1.1.1980). Also not nice, but gets the user to where it wants.
Any other solution that handles the exception, avoids the time-out, and exposes the bad luck to the user is fine, but will remain him w/o the desired file. One that is expected to be still useful.