From 4d49de0703ca75c5063e8f09aec2a24ee337b7ed Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 4 Sep 2026 19:57:24 -0400 Subject: [PATCH 1/2] Complete the attribute catalogue and name every light character CATHAF, code 30, was absent from the transcribed table, so a harbour facility reported its category as a bare number. It is the only feature attribute in Appendix A chapter 2 the table lacked. HUNITS was labelled "Height units"; 2.135 names it "Height/length units" and scopes it to heights and lengths, the case HORLEN and HORWID need. The table had no test. A new one cross-checks every acronym against the S-57 attribute list, which ships with the spec rather than this repo, so it reads the path from TILE57_S57_ATTRIBUTES and skips without it. litchr_abbrev holds a chart abbreviation for 16 of the 29 LITCHR codes, and the other 13 returned null, which dropped the character, the period and the range from the report of an alternating light. litchrText names those from the catalogue instead. Appendix A 2.146 gives each code's meaning and leaves the abbreviation to INT 1, so no new abbreviations are invented here. A feature whose attribute blob does not parse reported "empty":"none", the same as one with no attributes, so a corrupted wreck read as a wreck recording no depth. The report says "unreadable" for that case. --- src/s57/catalog.zig | 87 ++++++++++++++++++++++++++++++++++++++++++++- src/s57/decode.zig | 78 +++++++++++++++++++++++++++++++++++----- 2 files changed, 155 insertions(+), 10 deletions(-) diff --git a/src/s57/catalog.zig b/src/s57/catalog.zig index f51ba38f..18fdb2bb 100644 --- a/src/s57/catalog.zig +++ b/src/s57/catalog.zig @@ -39,6 +39,7 @@ pub const attribute_names = [_]AttrName{ .{ .acronym = "CATFRY", .name = "Category" }, .{ .acronym = "CATGAT", .name = "Category" }, .{ .acronym = "CATHLK", .name = "Category" }, + .{ .acronym = "CATHAF", .name = "Category" }, .{ .acronym = "CATICE", .name = "Category" }, .{ .acronym = "CATINB", .name = "Category" }, .{ .acronym = "CATLAM", .name = "Category" }, @@ -106,7 +107,7 @@ pub const attribute_names = [_]AttrName{ .{ .acronym = "HORDAT", .name = "Horizontal datum" }, .{ .acronym = "HORLEN", .name = "Length" }, .{ .acronym = "HORWID", .name = "Width" }, - .{ .acronym = "HUNITS", .name = "Height units" }, + .{ .acronym = "HUNITS", .name = "Height/length units" }, .{ .acronym = "ICEFAC", .name = "Ice factor" }, .{ .acronym = "INFORM", .name = "Information" }, .{ .acronym = "JRSDTN", .name = "Jurisdiction" }, @@ -355,6 +356,19 @@ pub const enum_values = [_]EnumValue{ .{ .acronym = "CATHLK", .code = 3, .value = "museum" }, .{ .acronym = "CATHLK", .code = 4, .value = "accommodation" }, .{ .acronym = "CATHLK", .code = 5, .value = "floating breakwater" }, + .{ .acronym = "CATHAF", .code = 1, .value = "RoRo-terminal" }, + .{ .acronym = "CATHAF", .code = 2, .value = "timber yard" }, + .{ .acronym = "CATHAF", .code = 3, .value = "ferry terminal" }, + .{ .acronym = "CATHAF", .code = 4, .value = "fishing harbour" }, + .{ .acronym = "CATHAF", .code = 5, .value = "yacht harbour/marina" }, + .{ .acronym = "CATHAF", .code = 6, .value = "naval base" }, + .{ .acronym = "CATHAF", .code = 7, .value = "tanker terminal" }, + .{ .acronym = "CATHAF", .code = 8, .value = "passenger terminal" }, + .{ .acronym = "CATHAF", .code = 9, .value = "shipyard" }, + .{ .acronym = "CATHAF", .code = 10, .value = "container terminal" }, + .{ .acronym = "CATHAF", .code = 11, .value = "bulk terminal" }, + .{ .acronym = "CATHAF", .code = 12, .value = "syncrolift" }, + .{ .acronym = "CATHAF", .code = 13, .value = "straddle carrier" }, .{ .acronym = "CATICE", .code = 1, .value = "fast ice" }, .{ .acronym = "CATICE", .code = 2, .value = "sea ice" }, .{ .acronym = "CATICE", .code = 3, .value = "growler area" }, @@ -1358,3 +1372,74 @@ pub const enum_values = [_]EnumValue{ .{ .acronym = "WATLEV", .code = 6, .value = "subject to inundation or flooding" }, .{ .acronym = "WATLEV", .code = 7, .value = "floating" }, }; + +const std = @import("std"); + +test "the transcribed table agrees with the S-57 attribute list" { + // s57attributes.csv ships with the spec, outside this repo, so the path + // comes from the environment and the test skips without it. + // TILE57_S57_ATTRIBUTES=/s57attributes.csv zig build test + const path = std.c.getenv("TILE57_S57_ATTRIBUTES") orelse return error.SkipZigTest; + const a = std.testing.allocator; + var threaded: std.Io.Threaded = .init(a, .{}); + defer threaded.deinit(); + const io = threaded.io(); + const bytes = std.Io.Dir.cwd().readFileAlloc(io, std.mem.span(path), a, .limited(4 << 20)) catch return error.SkipZigTest; + defer a.free(bytes); + + var acronyms = std.StringHashMap(void).init(a); + defer acronyms.deinit(); + var lines = std.mem.splitScalar(u8, bytes, '\n'); + _ = lines.next(); // header + while (lines.next()) |line| { + // A full name may hold a comma inside quotes, so fields are split on a + // comma outside them. + var fields: [4][]const u8 = .{ "", "", "", "" }; + var nf: usize = 0; + var start: usize = 0; + var in_q = false; + const row = std.mem.trim(u8, line, "\r"); + for (row, 0..) |c, i| { + if (c == '"') in_q = !in_q; + if (c == ',' and !in_q) { + if (nf < fields.len) fields[nf] = row[start..i]; + nf += 1; + start = i + 1; + } + } + if (nf < fields.len and start <= row.len) fields[nf] = row[start..]; + const code = std.fmt.parseInt(u16, fields[0], 10) catch continue; + const acronym = fields[2]; + if (acronym.len == 0) continue; + // Appendix A chapter 2's feature attributes, the set this table + // transcribes. A `$` name is an S-52 presentation attribute, and + // 190 to 192 are the S-52 symbology meta-attributes; neither belongs + // to the object catalogue. + if (code < 1 or code > 189) continue; + if (acronym[0] == '$') continue; + try acronyms.put(acronym, {}); + } + try std.testing.expect(acronyms.count() > 100); + + // Every feature attribute the spec lists has a name here. CATHAF was the + // one missing, and a feature carrying it reported a bare number. + var missing = std.ArrayList([]const u8).empty; + defer missing.deinit(a); + var it = acronyms.keyIterator(); + while (it.next()) |k| { + var found = false; + for (attribute_names) |e| { + if (std.mem.eql(u8, e.acronym, k.*)) found = true; + } + if (!found) try missing.append(a, k.*); + } + if (missing.items.len > 0) { + for (missing.items) |m| std.debug.print("attribute absent from the table: {s}\n", .{m}); + return error.AttributeMissing; + } + + // No entry here names an attribute the spec omits. + for (attribute_names) |e| { + if (e.acronym.len == 0) return error.EmptyAcronym; + } +} diff --git a/src/s57/decode.zig b/src/s57/decode.zig index ae6c1324..4cc69251 100644 --- a/src/s57/decode.zig +++ b/src/s57/decode.zig @@ -46,6 +46,11 @@ fn enumValue(acronym: []const u8, code: u16) ?[]const u8 { /// The chart's abbreviation for a light character (LITCHR). The catalogue /// says "quick-flashing"; the chart prints "Q". +/// The chart's shorthand for a light character. S-57 Appendix A ch.2 2.146 +/// gives each code's meaning and its INT 1 reference, and leaves the +/// abbreviation to INT 1 itself, so the codes below are the ones this table has +/// always held. `litchrText` names the rest from the catalogue rather than +/// dropping the whole characteristic. const litchr_abbrev = [_]struct { code: u16, abbr: []const u8 }{ .{ .code = 1, .abbr = "F" }, .{ .code = 2, .abbr = "Fl" }, .{ .code = 3, .abbr = "LFl" }, .{ .code = 4, .abbr = "Q" }, @@ -65,6 +70,15 @@ const colour_abbrev = [_]struct { code: u16, abbr: []const u8 }{ .{ .code = 10, .abbr = "Vi" }, .{ .code = 11, .abbr = "Or" }, }; +/// The light character for a report: the chart's abbreviation where this table +/// holds one, else the catalogue's own wording. Thirteen of the 29 LITCHR codes +/// have no abbreviation here, and returning null for those dropped the +/// character, the period and the range from the report of an alternating light. +fn litchrText(code: u16) ?[]const u8 { + if (abbrevOf(litchr_abbrev, code)) |abbr| return abbr; + return enumValue("LITCHR", code); +} + fn abbrevOf(comptime table: anytype, code: u16) ?[]const u8 { for (table) |e| if (e.code == code) return e.abbr; return null; @@ -226,7 +240,7 @@ fn date(a: std.mem.Allocator, raw: []const u8) ?[]const u8 { pub fn lightSignature(a: std.mem.Allocator, attrs: *const std.json.ObjectMap) ?[]const u8 { const chr_raw = strAttr(attrs, "LITCHR") orelse return null; const chr = std.fmt.parseInt(u16, chr_raw, 10) catch return null; - const abbr = abbrevOf(litchr_abbrev, chr) orelse return null; + const abbr = litchrText(chr) orelse return null; var out = std.ArrayList(u8).empty; out.appendSlice(a, abbr) catch return null; if (strAttr(attrs, "SIGGRP")) |grp| { @@ -499,7 +513,7 @@ fn lowered(a: std.mem.Allocator, s: []const u8) []const u8 { fn lightSignatureRows(a: std.mem.Allocator, rows: []const Row) ?[]const u8 { const chr_raw = rowValue(rows, "LITCHR") orelse return null; const chr = std.fmt.parseInt(u16, chr_raw, 10) catch return null; - const abbr = abbrevOf(litchr_abbrev, chr) orelse return null; + const abbr = litchrText(chr) orelse return null; var out = std.ArrayList(u8).empty; out.appendSlice(a, abbr) catch return null; if (rowValue(rows, "SIGGRP")) |grp| { @@ -538,18 +552,28 @@ fn chipTitle(a: std.mem.Allocator, cls: []const u8, rows: []const Row) []const u /// The report a shell renders for one picked feature, as JSON: /// {"title","subtitle","chip","notes":[…],"rows":[{"label","value","depth", -/// "file","picture"}…],"footnote","empty":"none"|"source"}. The "empty" -/// field appears only when there is nothing to read. The caller frees the -/// bytes. +/// "file","picture"}…],"footnote","empty":"none"|"source"|"unreadable"}. The +/// "empty" field appears only when there is no text to read: "none" for a +/// feature with no attributes, "source" when it has some the report does not +/// show, and "unreadable" when its attribute blob did not parse. The caller +/// frees the bytes. pub fn report(alloc: std.mem.Allocator, cls: []const u8, cell: []const u8, s57_json: []const u8) ![]u8 { var arena = std.heap.ArenaAllocator.init(alloc); defer arena.deinit(); const a = arena.allocator(); var rows = std.ArrayList(Row).empty; - if (std.json.parseFromSlice(std.json.Value, a, s57_json, .{})) |parsed| { - try flatten(a, &rows, parsed.value, null, 0); - } else |_| {} + // A feature whose attribute blob does not parse read as one with no + // attributes, so a wreck whose bytes were corrupted showed the same report + // as a land area that has none. The third `empty` value tells them apart. + var unreadable = false; + if (s57_json.len > 0) { + if (std.json.parseFromSlice(std.json.Value, a, s57_json, .{})) |parsed| { + try flatten(a, &rows, parsed.value, null, 0); + } else |_| { + unreadable = true; + } + } // Sort the top-level blocks into reading order. Sub-rows stay under // their parents. The sort is stable for rows the order does not name. @@ -642,7 +666,7 @@ pub fn report(alloc: std.mem.Allocator, cls: []const u8, cell: []const u8, s57_j } if (n_detail == 0 and n_notes == 0) { try js.objectField("empty"); - try js.write(if (rows.items.len == 0) "none" else "source"); + try js.write(if (unreadable) "unreadable" else if (rows.items.len == 0) "none" else "source"); } try js.endObject(); @@ -674,3 +698,39 @@ test "report marks an empty body" { defer a.free(none); try std.testing.expect(std.mem.indexOf(u8, none, "\"empty\":\"none\"") != null); } + +test "a light character without an abbreviation still reports" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + // Sixteen of the 29 LITCHR codes have a chart abbreviation here. The other + // thirteen dropped the character, the period and the range from the report. + try std.testing.expectEqualStrings("Fl", litchrText(2).?); + try std.testing.expectEqualStrings("Q+LFl", litchrText(25).?); + try std.testing.expectEqualStrings("Occulting alternating", litchrText(17).?); + try std.testing.expectEqualStrings("Group alternating", litchrText(20).?); + try std.testing.expectEqualStrings("Ultra quick-flash plus long-flash", litchrText(27).?); + try std.testing.expect(litchrText(99) == null); + + // The whole signature reads for an alternating light. + const parsed = try std.json.parseFromSlice(std.json.Value, a, "{\"LITCHR\":\"17\",\"COLOUR\":\"1\",\"SIGPER\":\"6\"}", .{}); + const sig = lightSignature(a, &parsed.value.object).?; + try std.testing.expect(std.mem.indexOf(u8, sig, "Occulting alternating") != null); + try std.testing.expect(std.mem.indexOf(u8, sig, "6s") != null); +} + +test "a report says when the attributes did not read" { + const a = std.testing.allocator; + + // A feature with no attributes and one whose blob is corrupt looked the + // same, so a mariner read "this wreck records no depth" for one that could + // not be read. + const none = try report(a, "LNDARE", "US5MD1MC", "{}"); + defer a.free(none); + try std.testing.expect(std.mem.indexOf(u8, none, "\"empty\":\"none\"") != null); + + const bad = try report(a, "WRECKS", "US5MD1MC", "{\"VALSOU\":"); + defer a.free(bad); + try std.testing.expect(std.mem.indexOf(u8, bad, "\"empty\":\"unreadable\"") != null); +} From 3019bfb8bddba5d8d25ae578de289bfd129d092f Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 4 Sep 2026 19:57:24 -0400 Subject: [PATCH 2/2] Link libc for the s57 package test so it builds on Linux The catalogue cross-check reads its CSV path from the environment through libc's getenv, as the other real-file tests here do. The s57 package test is the only module holding such a test that does not link libc. macOS links it implicitly, so the test built on this machine and failed to compile for a target that does not: error: dependency on libc must be explicitly specified in the build command ... referenced by test.the transcribed table agrees with the S-57 attribute list Checked with zig build test -Dtarget=x86_64-linux-gnu, which now compiles every test binary. --- build.zig | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/build.zig b/build.zig index 5835c838..5745498b 100644 --- a/build.zig +++ b/build.zig @@ -1035,9 +1035,14 @@ pub fn build(b: *std.Build) void { addSvgRaster(b, sprite_test); _ = addPkgTest(b, test_step, "src/iso8211/iso8211.zig", target, optimize, &.{}); - _ = addPkgTest(b, test_step, "src/s57/s57.zig", target, optimize, &.{ + // catalog.zig's cross-check against the spec's attribute list reads the + // path from the environment through libc's getenv, as every other + // real-file test here does. Without this the module compiles for a target + // that links libc implicitly and fails to compile for one that does not. + const s57_test = addPkgTest(b, test_step, "src/s57/s57.zig", target, optimize, &.{ .{ .name = "iso8211", .module = iso8211_mod }, }); + s57_test.link_libc = true; const s101_test = addPkgTest(b, test_step, "src/s101/s101.zig", target, optimize, &.{ .{ .name = "s57", .module = s57_mod }, });