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.

Sign up for GitHub

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

Jump to bottom

fix garmin serial/usb device character encoding/decoding. #1117

Merged
merged 12 commits into from
May 19, 2023
Merged

fix garmin serial/usb device character encoding/decoding. #1117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
37c1201
attempt to use correct codec with garmin reader.
tsteven4 May 15, 2023
c1d5baa
fix garmin writer wrt encoding.
tsteven4 May 15, 2023
47159d5
add garmin option to force codec.
tsteven4 May 15, 2023
1a82edb
Merge branch 'master' into garmincodec
tsteven4 May 15, 2023
2d5a206
update serialization ref files for garmin codec option.
tsteven4 May 15, 2023
9bea843
fix potential overwrite bug with route/track names
tsteven4 May 15, 2023
1e25ad2
fix up for old Qt.
tsteven4 May 15, 2023
9c1e1af
correct assertion
tsteven4 May 15, 2023
b4181cd
use static_assert to for fixed buffer size checks.
tsteven4 May 16, 2023
89b95cc
document garmin codec option
tsteven4 May 16, 2023
911ab99
don't clean rtept/trkpt names for newer devices.
tsteven4 May 19, 2023
ced87b9
Merge branch 'master' into garmincodec
tsteven4 May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 104 additions & 55 deletions garmin.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

*/

#include <cctype> // for isalpha, toupper
#include <cassert> // for assert
#include <climits> // for INT_MAX
#include <cmath> // for atan2, floor, sqrt
#include <csetjmp> // for setjmp
Expand All @@ -30,6 +30,7 @@

#include <QByteArray> // for QByteArray
#include <QChar> // for QChar
#include <QRegularExpression> // for QRegularExpression
#include <QString> // for QString
#include <QTextCodec> // for QTextCodec
#include <QVector> // for QVector
Expand Down Expand Up @@ -76,6 +77,7 @@ static char* deficon = nullptr;
static char* category = nullptr;
static char* categorybitsopt = nullptr;
static char* baudopt = nullptr;
static char* opt_codec = nullptr;
static int baud = 0;
static int categorybits;
static int receiver_must_upper = 1;
Expand All @@ -85,8 +87,9 @@ static Vecs::fmtinfo_t gpx_vec;

#define MILITANT_VALID_WAYPT_CHARS "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"

/* Technically, even this is a little loose as spaces arent allowed */
/* Technically, even this is a little loose as spaces aren't allowed */
static const char* valid_waypt_chars = MILITANT_VALID_WAYPT_CHARS " ";
static QRegularExpression invalid_char_re;

static
QVector<arglist_t> garmin_args = {
Expand Down Expand Up @@ -125,7 +128,12 @@ QVector<arglist_t> garmin_args = {
},
{
"baud", &baudopt, "Speed in bits per second of serial port (baud=9600)",
nullptr, ARGTYPE_INT, ARG_NOMINMAX, nullptr },
nullptr, ARGTYPE_INT, ARG_NOMINMAX, nullptr
},
{
"codec", &opt_codec, "override codec to use for device",
nullptr, ARGTYPE_STRING, ARG_NOMINMAX, nullptr
},

};

Expand All @@ -134,8 +142,22 @@ static int d103_icon_number_from_symbol(const QString& s);
static void garmin_fs_garmin_after_read(GPS_PWay way, Waypoint* wpt, int protoid);
static void garmin_fs_garmin_before_write(const Waypoint* wpt, GPS_PWay way, int protoid);

static QByteArray str_from_unicode(const QString& qstr) {return codec->fromUnicode(qstr);}
static QString str_to_unicode(const QByteArray& cstr) {return codec->toUnicode(cstr);}
static QByteArray str_from_unicode(const QString& qstr)
{
return codec->fromUnicode(qstr);
}
static QString str_to_unicode(const QByteArray& cstr)
{
return codec->toUnicode(cstr);
}

static void
write_char_string(char* dest, const char* source, size_t destsize)
robertlipe marked this conversation as resolved.
Show resolved Hide resolved
{
// we zero fill and always terminate within the dest buffer.
strncpy(dest, source, destsize - 1);
dest[destsize-1] = 0;
}

static void
rw_init(const QString& fname)
Expand Down Expand Up @@ -168,14 +190,14 @@ rw_init(const QString& fname)
if (baudopt) {
baud = strtol(baudopt, nullptr, 0);
switch (baud) {
case 9600:
case 19200:
case 38400:
case 57600:
case 115200:
break;
default:
fatal("Baud rate %d is not supported\n", baud);
case 9600:
case 19200:
case 38400:
case 57600:
case 115200:
break;
default:
fatal("Baud rate %d is not supported\n", baud);
}
}

Expand All @@ -184,7 +206,7 @@ rw_init(const QString& fname)
}

/*
* THis is Gross. The B&W Vista sometimes sets its time decades into
* This is Gross. The B&W Vista sometimes sets its time decades into
* the future with no way to reset it. This apparently can "cure"
* an affected unit.
*/
Expand Down Expand Up @@ -244,8 +266,7 @@ rw_init(const QString& fname)
case 696: /* eTrex HC */
case 574: /* Geko 201 */
receiver_short_length = 6;
valid_waypt_chars =
MILITANT_VALID_WAYPT_CHARS " +-";
valid_waypt_chars = MILITANT_VALID_WAYPT_CHARS " +-";
setshort_badchars(mkshort_handle, "\"$.,'!");
break;

Expand Down Expand Up @@ -328,7 +349,7 @@ rw_init(const QString& fname)
}

/*
* Until Garmins documents how to determine valid character space
* Until Garmin documents how to determine valid character space
* for the new models, we just release this safety check manually.
*/
if (receiver_must_upper) {
Expand All @@ -347,7 +368,26 @@ rw_init(const QString& fname)
* However, this is still used for garmin_fs_garmin_after_read,
* garmin_fs_garmin_before_write.
*/
if (opt_codec != nullptr) {
// override expected codec with user supplied choice.
receiver_charset = opt_codec;
robertlipe marked this conversation as resolved.
Show resolved Hide resolved
}
codec = get_codec(receiver_charset);
if (global_opts.verbose_status) {
fprintf(stdout, "receiver charset detected as %s.\r\n", receiver_charset);
}

/*
* Beware, valid_waypt_chars shouldn't contain any character class metacharacters,
* i.e. '\', '^', '-', '[', or ']'
*/
assert(!QString(valid_waypt_chars).contains('\\'));
assert(!QString(valid_waypt_chars).contains('^'));
assert(!QString(valid_waypt_chars).contains('-'));
assert(!QString(valid_waypt_chars).contains('['));
assert(!QString(valid_waypt_chars).contains(']'));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could be a single QString::contains(const QRegularExpression &re, but it would admittedly be a quoting nightmare and wouldn't give you a single error message like this will. Your call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and it would save a bunch of QString construction. But, the code is only run on init, and only in debug builds, so I thought clarity was better.

invalid_char_re = QRegularExpression(QStringLiteral("[^%1]").arg(valid_waypt_chars));
assert(invalid_char_re.isValid());
robertlipe marked this conversation as resolved.
Show resolved Hide resolved
}

static void
Expand Down Expand Up @@ -383,7 +423,7 @@ rw_deinit()
}

static int
waypt_read_cb(int total_ct, GPS_PWay* )
waypt_read_cb(int total_ct, GPS_PWay*)
{
if (global_opts.verbose_status) {
static int i;
Expand Down Expand Up @@ -418,8 +458,8 @@ waypt_read()
for (int i = 0; i < n; i++) {
auto* wpt_tmp = new Waypoint;

wpt_tmp->shortname = QString::fromLatin1(way[i]->ident);
wpt_tmp->description = QString::fromLatin1(way[i]->cmnt);
wpt_tmp->shortname = str_to_unicode(way[i]->ident);
robertlipe marked this conversation as resolved.
Show resolved Hide resolved
wpt_tmp->description = str_to_unicode(way[i]->cmnt);
wpt_tmp->shortname = wpt_tmp->shortname.simplified();
wpt_tmp->description = wpt_tmp->description.simplified();
wpt_tmp->longitude = way[i]->lon;
Expand All @@ -433,8 +473,8 @@ waypt_read()
}
/*
* If a unit doesn't store altitude info (i.e. a D103)
* gpsmem will default the alt to INT_MAX. Other units
* (I can't recall if it was the V (D109) hor the Vista (D108)
* gpsmem will default the alt to INT_MAX. Other units
* (I can't recall if it was the V (D109) or the Vista (D108)
* return INT_MAX+1, contrary to the Garmin protocol doc which
* says they should report 1.0e25. So we'll try to trap
* all the cases here. Yes, libjeeps should probably
Expand Down Expand Up @@ -532,7 +572,7 @@ track_read()
if (trk_head == nullptr || array[i]->ishdr) {
trk_head = new route_head;
trk_head->rte_num = trk_num;
trk_head->rte_name = QString::fromLatin1(trk_name);
trk_head->rte_name = str_to_unicode(trk_name);
trk_num++;
track_add_head(trk_head);
}
Expand All @@ -554,7 +594,7 @@ track_read()
wpt->altitude = array[i]->alt;
wpt->heartrate = array[i]->heartrate;
wpt->cadence = array[i]->cadence;
wpt->shortname = array[i]->trk_ident;
wpt->shortname = str_to_unicode(array[i]->trk_ident);
wpt->SetCreationTime(array[i]->Time);
wpt->wpt_flags.is_split = checkWayPointIsAtSplit(wpt, laps,
nlaps);
Expand Down Expand Up @@ -611,7 +651,7 @@ route_read()
rte_head = new route_head;
route_add_head(rte_head);
if (csrc) {
rte_head->rte_name = QString::fromLatin1(csrc);
rte_head->rte_name = str_to_unicode(csrc);
}
} else {
if (array[i]->islink) {
Expand All @@ -620,7 +660,7 @@ route_read()
auto* wpt_tmp = new Waypoint;
wpt_tmp->latitude = array[i]->lat;
wpt_tmp->longitude = array[i]->lon;
wpt_tmp->shortname = array[i]->ident;
wpt_tmp->shortname = str_to_unicode(array[i]->ident);
route_add_wpt(rte_head, wpt_tmp);
}
}
Expand Down Expand Up @@ -657,8 +697,8 @@ pvt2wpt(GPS_PPvt_Data pvt, Waypoint* wpt)
* reference clocks.
*/
double wptime = 631065600.0 + pvt->wn_days * 86400.0 +
pvt->tow
- pvt->leap_scnds;
pvt->tow
- pvt->leap_scnds;
double wptimes = floor(wptime);
wpt->SetCreationTime(wptimes, 1000000.0 * (wptime - wptimes));

Expand Down Expand Up @@ -870,34 +910,40 @@ waypoint_prepare()
* cleaning
*/
char* ident = mkshort(mkshort_handle,
global_opts.synthesize_shortnames ? CSTRc(src) :
CSTRc(wpt->shortname), false);
global_opts.synthesize_shortnames ?
str_from_unicode(src).constData() :
str_from_unicode(wpt->shortname).constData(),
false);
/* Should not be a strcpy as 'ident' isn't really a C string,
* but rather a garmin "fixed length" buffer that's padded
* to the end with spaces. So this is NOT (strlen+1).
*/
memcpy(tx_waylist[i]->ident, ident, strlen(ident));
write_char_string(tx_waylist[i]->ident, ident, sizeof(tx_waylist[i]->ident));

if (global_opts.synthesize_shortnames) {
xfree(ident);
}
tx_waylist[i]->ident[sizeof(tx_waylist[i]->ident)-1] = 0;

// If we were explicitly given a comment from GPX, use that.
// This logic really is horrible and needs to be untangled.
if (!wpt->description.isEmpty() &&
global_opts.smart_names && !wpt->gc_data->diff) {
memcpy(tx_waylist[i]->cmnt, CSTRc(wpt->description), strlen(CSTRc(wpt->description)));
write_char_string(tx_waylist[i]->cmnt,
str_from_unicode(wpt->description).constData(),
sizeof(tx_waylist[i]->cmnt));
} else {
if (global_opts.smart_names &&
wpt->gc_data->diff && wpt->gc_data->terr) {
static_assert(sizeof(obuf) >= sizeof(tx_waylist[i]->cmnt));
snprintf(obuf, sizeof(obuf), "%s%u/%u %s",
get_gc_info(wpt),
wpt->gc_data->diff, wpt->gc_data->terr,
CSTRc(src));
memcpy(tx_waylist[i]->cmnt, obuf, strlen(obuf));
str_from_unicode(src).constData());
write_char_string(tx_waylist[i]->cmnt, obuf, sizeof(tx_waylist[i]->cmnt));
} else {
memcpy(tx_waylist[i]->cmnt, CSTRc(src), strlen(CSTRc(src)));
write_char_string(tx_waylist[i]->cmnt,
str_from_unicode(src).constData(),
sizeof(tx_waylist[i]->cmnt));
}
}

Expand Down Expand Up @@ -972,9 +1018,9 @@ route_hdr_pr(const route_head* rte)
(*cur_tx_routelist_entry)->rte_num = rte->rte_num;
(*cur_tx_routelist_entry)->isrte = 1;
if (!rte->rte_name.isEmpty()) {
strncpy((*cur_tx_routelist_entry)->rte_ident, CSTRc(rte->rte_name),
sizeof((*cur_tx_routelist_entry)->rte_ident) - 1);
(*cur_tx_routelist_entry)->rte_ident[sizeof((*cur_tx_routelist_entry)->rte_ident) - 1] = 0;
write_char_string((*cur_tx_routelist_entry)->rte_ident,
str_from_unicode(rte->rte_name).constData(),
sizeof((*cur_tx_routelist_entry)->rte_ident));
}
}

Expand Down Expand Up @@ -1009,23 +1055,24 @@ route_waypt_pr(const Waypoint* wpt)
rte->alt = 0;
}

char* d = rte->ident;
for (auto idx : wpt->shortname) {
int c = idx.toLatin1();
if (receiver_must_upper && isalpha(c)) {
c = toupper(c);
}
if (strchr(valid_waypt_chars, c)) {
*d++ = c;
}
QString cleanname = wpt->shortname;
/*
* Until Garmin documents how to determine valid character space
* for the new models, we just release this safety check manually.
*/
if (receiver_must_upper) {
cleanname = cleanname.toUpper().remove(invalid_char_re);
}
write_char_string(rte->ident,
str_from_unicode(cleanname).constData(),
sizeof(rte->ident));

rte->ident[sizeof(rte->ident)-1] = 0;
if (wpt->description.isEmpty()) {
rte->cmnt[0] = 0;
} else {
strncpy(rte->cmnt, CSTR(wpt->description), sizeof(rte->cmnt) - 1);
rte->cmnt[sizeof(rte->cmnt)-1] = 0;
write_char_string(rte->cmnt,
str_from_unicode(wpt->description).constData(),
sizeof(rte->cmnt));
}
cur_tx_routelist_entry++;
}
Expand All @@ -1051,8 +1098,9 @@ track_hdr_pr(const route_head* trk_head)
{
(*cur_tx_tracklist_entry)->ishdr = true;
if (!trk_head->rte_name.isEmpty()) {
strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(trk_head->rte_name), sizeof((*cur_tx_tracklist_entry)->trk_ident) - 1);
(*cur_tx_tracklist_entry)->trk_ident[sizeof((*cur_tx_tracklist_entry)->trk_ident)-1] = 0;
write_char_string((*cur_tx_tracklist_entry)->trk_ident,
str_from_unicode(trk_head->rte_name).constData(),
sizeof((*cur_tx_tracklist_entry)->trk_ident));
} else {
snprintf((*cur_tx_tracklist_entry)->trk_ident, sizeof((*cur_tx_tracklist_entry)->trk_ident), "TRACK%02d", my_track_count);
}
Expand All @@ -1068,8 +1116,9 @@ track_waypt_pr(const Waypoint* wpt)
(*cur_tx_tracklist_entry)->alt = (wpt->altitude != unknown_alt) ? wpt->altitude : 1e25;
(*cur_tx_tracklist_entry)->Time = wpt->GetCreationTime().toTime_t();
if (!wpt->shortname.isEmpty()) {
strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(wpt->shortname), sizeof((*cur_tx_tracklist_entry)->trk_ident) - 1);
(*cur_tx_tracklist_entry)->trk_ident[sizeof((*cur_tx_tracklist_entry)->trk_ident)-1] = 0;
write_char_string((*cur_tx_tracklist_entry)->trk_ident,
str_from_unicode(wpt->shortname).constData(),
sizeof((*cur_tx_tracklist_entry)->trk_ident));
}
(*cur_tx_tracklist_entry)->tnew = wpt->wpt_flags.new_trkseg;
cur_tx_tracklist_entry++;
Expand Down
2 changes: 2 additions & 0 deletions reference/format3.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ option garmin bitscategory Bitmap of categories integer 1 65535 https://www.gps

option garmin baud Speed in bits per second of serial port (baud=9600) integer https://www.gpsbabel.org/WEB_DOC_DIR/fmt_garmin.html#fmt_garmin_o_baud

option garmin codec override codec to use for device string https://www.gpsbabel.org/WEB_DOC_DIR/fmt_garmin.html#fmt_garmin_o_codec

file r-rw-- gtrnctr tcx/crs/hst/xml Garmin Training Center (.tcx/.crs/.hst/.xml) gtrnctr
https://www.gpsbabel.org/WEB_DOC_DIR/fmt_gtrnctr.html
option gtrnctr course Write course rather than history, default yes boolean 1 https://www.gpsbabel.org/WEB_DOC_DIR/fmt_gtrnctr.html#fmt_gtrnctr_o_course
Expand Down
1 change: 1 addition & 0 deletions reference/help.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ File Types (-i and -o options):
category Category number to use for written waypoints
bitscategory Bitmap of categories
baud Speed in bits per second of serial port (baud=9600
codec override codec to use for device
tsteven4 marked this conversation as resolved.
Show resolved Hide resolved
gtrnctr Garmin Training Center (.tcx/.crs/.hst/.xml)
course (0/1) Write course rather than history, default yes
sport Sport: Biking (deflt), Running, MultiSport, Other
Expand Down
16 changes: 16 additions & 0 deletions xmldoc/formats/options/garmin-codec.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<para>
This lets you override the default codec used for your device when reading or writing
strings from or to your Garmin device. The default codec is device dependent, you can see
what codec is being used with your device by adding the -vs option to the command line.
</para>
<para>
<userinput>
gpsbabel -vs -w -i garmin -f usb: -o gpx -F garmin.gpx
</userinput>
<userinput>
gpsbabel -w -i garmin,codec=windows-1251 -f usb: -o gpx -F garmin.gpx
</userinput>
<userinput>
gpsbabel -w -i gpx -f garmin.gpx -o garmin,codec=windows-1251 -F usb:
</userinput>
</para>