GPSBabel / gpsbabel

GPSBabel: convert, manipulate, and transfer data from GPS programs or GPS receivers. Open Source and supported on MacOS, Windows, Linux, and more. Pointy clicky GUI or a command line version...
https://www.gpsbabel.org
GNU General Public License v2.0
470 stars 125 forks source link

consolidate handling of extensions in gpx writer. #1200

Closed tsteven4 closed 10 months ago

tsteven4 commented 10 months ago

This is a user visible change.

Previously if gARmIN sPECIAL dATA existed and both garminextensions and humminbirdextensions were false then the gpx writer would write a subset of it (and not passthrough any gpx -> gpx data.) If gARmIN sPECIAL dATA existed and either extension option was true, then a different subset would be written (and any gpx -> gpx data would not be passed through.) Now the garminextensions option must used to have any gARmIN sPECIAL dATA data written.

Previously, when passing through extension data from the gpx reader to the gpx writer, some elements of the garmin and humminbird extensions were not included. Now, all foreign elements are included. As before, passthrough is only used when both extension options are false.

tsteven4 commented 10 months ago

@robertlipe are you ok with the user interface changes here? The mode where we found gmsd data and automatically wrote it is gone, it's either passthrough from gpx -> gpx of unknown extension data, or creation of extension data with the garminextension option now.

GPSBabelDeveloper commented 10 months ago

Doesn't this mean if we read from a receiver that has, say categories (e.g. streetpilot) we won't get Garmin extensions in the GPX?

Maybe that's a generation gap that's not really very relevant. It was a pretty small window of receivers.

Even if the answer is yes and you think it's justified, to for it.

On Sun, Oct 29, 2023, 5:24 PM tsteven4 @.***> wrote:

@robertlipe https://github.com/robertlipe are you ok with the user interface changes here? The mode where we found gmsd data and automatically wrote it is gone, it's either passthrough from gpx -> gpx of unknown extension data, or creation of extension data with the garminextension option now.

— Reply to this email directly, view it on GitHub https://github.com/GPSBabel/gpsbabel/pull/1200#issuecomment-1784246774, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC3VADZUZ2PFZZ3BM5JMP2LYB3JSVAVCNFSM6AAAAAA6TL3QWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBUGI2DMNZXGQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

tsteven4 commented 10 months ago

The answer is yes if you don't use -o,garminextensions, and no otherwise.

A related test is in garmin_gpi.test where we had to add the garminextensions option

-gpsbabel -i garmin_gpi -f ${REFERENCE}/umsonstdraussen.gpi -o gpx,gpxver=1.1 -F ${TMPDIR}/umsonstdraussen.gpx
+gpsbabel -i garmin_gpi -f ${REFERENCE}/umsonstdraussen.gpi -o gpx,garminextensions -F ${TMPDIR}/umsonstdraussen.gpx

which produces similar output. I understand the namespace declaration changes, but hidden in there we do handle a 0x1a substitute character differently.

robertlipe commented 10 months ago

I'm totally down with that.

Go for it.

On Sun, Oct 29, 2023 at 6:47 PM tsteven4 @.***> wrote:

The answer is yes if you don't use -o,garminextensions, and no otherwise.

A related test is in garmin_gpi.test where we had to add the garminextensions option

-gpsbabel -i garmin_gpi -f ${REFERENCE}/umsonstdraussen.gpi -o gpx,gpxver=1.1 -F ${TMPDIR}/umsonstdraussen.gpx +gpsbabel -i garmin_gpi -f ${REFERENCE}/umsonstdraussen.gpi -o gpx,garminextensions -F ${TMPDIR}/umsonstdraussen.gpx

which produces similar output. I understand the namespace declaration changes, but hidden in there we do handle a 0x1a substitute character differently.

— Reply to this email directly, view it on GitHub https://github.com/GPSBabel/gpsbabel/pull/1200#issuecomment-1784277506, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD36KG625VMRSJKZ2NUDYB3TI3AVCNFSM6AAAAAA6TL3QWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBUGI3TONJQGY . You are receiving this because you were mentioned.Message ID: @.***>

tsteven4 commented 10 months ago

I suspect umsonstdraussen.gpx was really generated when Qt didn't strip illegal characters in qxmlstreamwriter so we did in gpsbabel::xmlstreamawriter. We substituted a space. It looks like Qt just drops them. The writer does set error status after we write the text with the 0x1e character, but we aren't looking for the error, and indeed in this case we would want to ignore it.

robertlipe commented 10 months ago

Very likely. Since the Creator tag says it's ours, we get to define truth in that file.

We've had a few other cases where we get control characters in source files and never really know what to do with them in GPX or KML or other files that don't really allow such things. Cleansing those is another area where we've wandered around with a variety of solutions through the years.

We'll probably discover a few more less-than-perfect ways to do it.

RJL

On Sun, Oct 29, 2023 at 7:26 PM tsteven4 @.***> wrote:

I suspect umsonstdraussen.gpx was really generated when Qt didn't strip illegal characters in qxmlstreamwriter so we did in gpsbabel::xmlstreamawriter. We substituted a space. It looks like Qt just drops them. The writer does set error status after we write the text with the 0x1e character, but we aren't looking for the error, and indeed in this case we would want to ignore it.

— Reply to this email directly, view it on GitHub https://github.com/GPSBabel/gpsbabel/pull/1200#issuecomment-1784300003, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD32GSEFCFFUNFCNJAITYB3X4TAVCNFSM6AAAAAA6TL3QWGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBUGMYDAMBQGM . You are receiving this because you were mentioned.Message ID: @.***>