-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
minmea: add new recipe #24546
minmea: add new recipe #24546
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Feel free to ping me once you resolve the compilation issues @toge! :) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
This nowadays has less sense than before with the addition of the [platform_tool_requires] in Conan 2, but Conan 1 still needs a way to specify that they already have pkgconf :)
#include "minmea.h" | ||
|
||
char* nmea_data[] = { | ||
"$GPGGA,155246.585,5231.171,N,01321.830,E,1,12,1.0,0.0,M,0.0,M,,*6F", |
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.
Since there has been a shift towards simpler test_package executables lately, this one could probably be reduced to parsing just a single NMEA string as a test as well.
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.
OK. I am going to make test_package.c simpler.
recipes/minmea/all/conanfile.py
Outdated
|
||
def validate(self): | ||
if is_msvc(self): | ||
raise ConanInvalidConfiguration(f"{self.ref} can not be built on Visual Studio and msvc.(yet)") |
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.
What is breaking the build on MSVC?
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.
I missed this sentences in README.md.
https://github.com/kosma/minmea/blob/e368d847d75abd891c651f7d30ce5efb6681adb7/README.md#compatibility
I will try to support MSVC.
Co-authored-by: Abril Rincón Blanco <[email protected]>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@AbrilRBS |
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.
The version name needs adjusting, otherwise LGTM.
Co-authored-by: Martin Valgur <[email protected]>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Hello @toge, I applied few changed to this PR:
Build log: Details
|
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Uilian Ries <[email protected]>
Conan v1 pipeline ✔️All green in build 14 (
Conan v2 pipeline ✔️
All green in build 14 (
|
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.
LGTM.
The new version format is good for this PR:
- It's a new fresh recipe in CCI
- We don't expect a new release for minmea soon
- Using Conan version range for this recipe in CCI is most unlikely
Summary
Changes to recipe: minmea/*
Motivation
The minmea is a library for parsing GPS NMEA data.
Details
minmea has a stable release, but it is too old (2014).
MINMEA is in continuous development until 2023.