-
Notifications
You must be signed in to change notification settings - Fork 10
Added support to cache online tile maps, also added custom http heade… #41
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.
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
base: master
Are you sure you want to change the base?
Conversation
src/index.ts
Outdated
| }) | ||
| if (!response.ok) { | ||
| console.error(`Error fetching tile ${provider.name}/${z}/${x}/${y}:`) | ||
| res.sendStatus(500) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we pass on whatever status code we got from upstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mostly worked, except when I entered a non working chart source the result was this:

This feature is a bit dangerous, as it can fill up your disk and effectively lock people out of their system if the system won't start properly with disk full.
I propose at least the following safeguards:
- minimum free disk space - so that seeding stops if there is not enough free space
- some threshold for seeding region size, maybe with just an alert to confirm that the region is large
This kind of long running operations easily error out for one reason or another, and you don't want to start over from the very beginning. The jobs are not persisted over restarts, so it would be useful if the UI could present a list of cached providers and their seeding statuses: what regions are seeded and their status, for example in terms of number of files on disk and a [Continue seeding] button, that would create the tiles list, check for their existence and fetch only missing tiles.
public/index.html
Outdated
| <label for="chart">Chart:</label> | ||
| <select id="chart" name="chart" required> | ||
| <option value="">-- Select chart --</option> | ||
| <option value="eniro">Eniro</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these defaults used in some cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will remove them
public/index.html
Outdated
| <option value="24">24</option> | ||
| </select> | ||
| <button id="seedChartsButton">Seed Chart</button> | ||
| <button id="clearCacheButton">Clear Cache</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is all right like this to get started, but I think it would make sense to have a separate list of cached chart providers with associated [Clear Cache] functionality and a separate panel/control for starting seeding jobs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some how tried to fix it
src/chartDownloader.ts
Outdated
| const bbox = this.getBBox(feature.geometry as Polygon); | ||
| for (let z = zoomMin; z <= zoomMax; z++) { | ||
| const [minX, minY] = this.lonLatToTileXY(bbox[0], bbox[3], z); // top-left | ||
| const [maxX, maxY] = this.lonLatToTileXY(bbox[2], bbox[1], z); // bottom-right |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this work over antimeridian?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, I hope
|
|
||
| async getRegion(regionGUID: string): Promise<Record<string, any>> { | ||
| let regionData: any | ||
| const resp = await fetch(`${this.urlBase}/signalk/v2/api/resources/regions/${regionGUID}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@panaaj do we have a way to call the resources api from a plugin, to skip the http call?
src/index.ts
Outdated
| const { jobId, downloader } = ChartDownloader.createAndRegister(urlBase, defaultChartsPath, provider) | ||
| // Long going process, create an endpoint to monitor progress | ||
| ;(async () => { | ||
| await downloader.deleteTiles(regionGUID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a weird construction. Can't deleteTiles start deleting the files in the background and return a Promise, and here we just don't await it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to tidy this up a little
…rs to pass to the remote tile server
37dba5a to
f16dd19
Compare

Added support to cache online tile maps,
Added custom http headers to pass to the remote tile server to pass around CORS limitations