search

mikalhart / TinyGPSPlus

A new, customizable Arduino NMEA parsing library
http://arduiniana.org
1.05k stars 486 forks source link

Arduino library compatibility #92

Closed santaimpersonator closed 2 years ago

santaimpersonator commented 3 years ago

As mentioned in this issue (https://github.com/mikalhart/TinyGPSPlus/issues/85), the library name and file names don't comply with Arduino's specifications:

Library names must contain only basic letters (A-Z or a-z) and numbers (0-9), spaces ( ), underscores (_), dots (.) and dashes (-).

KenwoodFox commented 2 years ago

Can we get this merged?

mikalhart commented 2 years ago

I like this idea, but first a question. Is there any way of accomplishing this without breaking every TinyGPS++ sketch ever written? I thought to try creating a dummy TinyGPSPlus.h file that simply #included the (current) TinyGPS++.h, but I'm not sure this completely solves the problem, does it?

per1234 commented 2 years ago

The only change that is required in order to be accepted into the Arduino Library Manager index is to make the name value in library.properties compliant with the requirements mentioned in the Arduino library specification: https://arduino.github.io/arduino-cli/latest/library-specification/#libraryproperties-file-format

name - the name of the library. Library names must contain only basic letters (A-Z or a-z) and numbers (0-9), spaces ( ), underscores (_), dots (.) and dashes (-). They must start with a letter or number. They must contain at least one letter. Note that libraries with a name value starting with Arduino will no longer be allowed addition to the Library Manager index as these names are now reserved for official Arduino libraries.

Once a library is in Library Manager, changing this name value is more significant because it is the unique identifier used to refer to the library in machine operations (e.g., arduino-cli lib install TinyGPSPlus), but for a library not yet in the index, the only current use of the name value is to identify the library to the user (e.g., File > Examples > TinyGPS++). That could also be seen as breaking because it might make some tutorials outdated, but I don't think it will have too serious of an impact on the users.

In addition to checking the libraries for compliance with the hard requirements that allow them to play nicely with the system, the Arduino Lint tool used by the system also runs checks for best practices. Violations of these result in a warning. There is no requirement to resolve these warnings. They are only suggestions for possible improvements.

One of these suggestions is for the library to contain a header file matching the library name. The library name determines the Library Manager installation folder name and header/library folder name matching is a factor in dependency resolution when two installed libraries contain a header matching an #include directive in the code being compiled. So it is a good thing to do, but it is not required. I think the approach of adding a wrapper header is the right thing to do.

santaimpersonator commented 2 years ago

@per1234 Thanks for the insight. @mikalhart Sorry, I didn't consider that.

Let me know if you'd prefer that I create a new pull request from a branch with the commit history cleaned up.

santaimpersonator commented 2 years ago

@mikalhart Just wanted to double check if the new changes work for you.

per1234 commented 2 years ago

Hi @mikalhart. I just wanted to check in with you to see whether you have had a chance to look at this proposal.

If you have any questions or concerns regarding the Arduino Library Manager, I'll be happy to assist.

per1234 commented 2 years ago

Update: I just noticed that while this proposal was pending another library claimed the "TinyGPSPLUS" library name in the Arduino Library Manager registry: https://github.com/Tinyu-Zhao/TinyGPSPlus

Since it is the sole unique identifier of the library, each library must have a unique name (case insensitive). So you will need to pick another one that has not yet been taken.

santaimpersonator commented 2 years ago

@per1234 That library (https://github.com/Tinyu-Zhao/TinyGPSPlus) appears to be a fork of this repository. If @mikalhart decides to incorporate these changes, could we overwrite the forked library with this one?

Otherwise, I can leave a note in issue https://github.com/mikalhart/TinyGPSPlus/issues/85 about the "TinyGPSPLUS" library name being taken and close all the related pull requests, until another library name is decided on.

TD-er commented 2 years ago

For PlatformIO library naming, you can prefix the name with the Github account, just like in the url here.

Not sure how Arduino libraries are managed, but you could simply create a name with the Github name prefixed?

per1234 commented 2 years ago

could we overwrite the forked library with this one?

The only way to gain possession of the name would be for the owner to request Arduino to either remove their library from the Library Manager or else to change the name it is registered under. You would need to work that out with the current owner of the name.

you could simply create a name with the Github name prefixed?

Here are the naming requirements:

https://arduino.github.io/arduino-cli/latest/library-specification/#libraryproperties-file-format

Library names must contain only basic letters (A-Z or a-z) and numbers (0-9), spaces ( ), underscores (_), dots (.) and dashes (-). They must start with a letter or number. They must contain at least one letter. Note that libraries with a name value starting with Arduino will no longer be allowed addition to the Library Manager index as these names are now reserved for official Arduino libraries.

So you are welcome to use any name that meets those requirements and is not already taken.

santaimpersonator commented 2 years ago

@per1234 @mikalhart

I'll close this pull request and https://github.com/arduino/library-registry/pull/110; and leave issue https://github.com/mikalhart/TinyGPSPlus/issues/85 open with a note about the library name being taken. I am happy to submit new pull requests, when a library name is chosen.

svdrummer commented 2 years ago

I saw discussions on adding the name to Arduino. IT went on for months, so the other guy just went ahead and placed it in the arduino library repository. It wasn't done in secret or anything...just a guy being proactive.

TD-er commented 2 years ago

Yep, it may be done with good intentions, but now there is the problem that the one that is in the Arduino library manager is not the one of the main repository. So now there is a dependency on a fork to keep the one in the lib manager up-to-date.

santaimpersonator commented 2 years ago

Reopening pull request in regards to checklist: https://github.com/mikalhart/TinyGPSPlus/issues/85#issuecomment-1008245696

mikalhart commented 2 years ago

Thanks to all of you for the attention to this issue. @santaimpersonator @per1234