Correct the bbox coordinate order and document the time parameters - #40
Merged
Conversation
bbox is parsed latitude-first, but README, the code comment and the OpenAPI description in #10 all said lon1,lat1,lon2,lat2. Following the docs produced a box with a latitude of 130, which cannot exist — so the query was accepted and silently matched nothing rather than failing. The README example was Australia written in the order that does not work. Correct the documentation to match the code rather than the reverse: the current behaviour is what any working caller already relies on, and changing the parsing would break them silently, in the same way the wrong docs do now. Whether lat-first is the right choice for the v2 API is a question for SignalK/signalk-server#2504, not something to settle unilaterally here. Also document /self/track and the from/to/duration/resolution parameters added in #37, which had no user-facing documentation at all. Every example in the README is now executed by readme.test.ts, including the antimeridian case, so a wrong example fails the build instead of silently misleading people.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bboxis parsed latitude-first, but every piece of documentation said longitude-first.lon1, lat1, lon2, lat2validateParameterslon1,lat1,lon2,lat2west,south,east,northsw: [swLat, swLng], ne: [neLat, neLng]The failure mode is the bad kind: quiet. The README's own example,
?bbox=130,-35,139,-33, parses as a box starting at latitude 130 — impossible, so it matches nothing and returns{}with a 200. No error, no clue.#10's OpenAPI example gives the confusion away:
51.28,-0.489,…is London as lat, lon, sitting directly under text that sayswest,south,east,north.Docs changed, code unchanged
Anyone with a working
bboxquery today is using lat-first — that is what the code does. Flipping the parser to match the docs would break them exactly as silently as the docs break newcomers. So the documentation moves.Whether lat-first is the right choice at all is a spec question for signalk-server#2504 — the v2 API can settle it deliberately. Note the parameter is the odd one out: this endpoint's own GeoJSON output is
[lng, lat].The antimeridian case is now documented too (
lon1greater thanlon2, e.g.bbox=-10,175,10,-175), which the code has always supported via the+360increateInBoundsbut nothing mentioned.Also: the time parameters had no documentation
/self/trackandfrom/to/duration/resolutionshipped in #37 with nothing user-facing. Added, including the half-open window semantics that let clients concatenate consecutive bands safely, and a note thattimespan/timespanOffsetare Freeboard-SK compatibility rather than spec.Every example is now executed
src/readme.test.tsruns each README example as a test, the antimeridian box included. A wrong example now fails the build instead of quietly misleading someone — which is the actual mechanism that prevents this recurring.79 tests pass; typecheck, lint, format and build clean.