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

Improve diagnostics when no parser modules are loaded #51

Open
igrr opened this issue May 14, 2021 · 0 comments
Open

Improve diagnostics when no parser modules are loaded #51

igrr opened this issue May 14, 2021 · 0 comments

Comments

@igrr
Copy link
Collaborator

igrr commented May 14, 2021

I've ran into this more than a few times already during development, that when NMEA_PARSER_PATH is not set correctly, no modules are loaded. In this situation, parsing of any NMEA message silently fails.

This is perfectly legal, but sets one on a somewhat lengthy troubleshooting path: why didn't the message get parsed? why didn't the message type get detected? why no parsers loaded? ah, because shared libraries haven't been found!.

What doesn't help is that the parser loading happens from a constructor function, where it isn't possible to do anything about the return value of nmea_load_parsers:

libnmea/src/nmea/nmea.c

Lines 81 to 85 in d55d296

void __attribute__ ((constructor)) nmea_init(void);
void nmea_init()
{
nmea_load_parsers();
}

In my opinion, the best solution would be to deprecate the automatic loading of parsers, and let the application call nmea_init and nmea_cleanup explicitly, as it is common for C libraries. Then an error code could be returned to the application from nmea_init, indicating for example that no parsers were loaded. This would be a breaking change for existing projects, though. Given that libnmea is pre-1.0, this might still be acceptable, assuming that semver is followed?

The next best solution seems to be to add a function nmea_get_parser_count or similar, which would return the number of parsers loaded. The application can then check if the value is zero and bail out with a human-readable error message if it is.

I can submit a PR for either of the solutions, but would like to get @jacketizer's advice and approval first.

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

No branches or pull requests

1 participant