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.

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

Add File watcher and Mapbox Style support #34

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

panaaj
Copy link
Member

@panaaj panaaj commented Sep 27, 2024

This PR includes the following changes:

  1. Adds a file watcher to charts folders to detect new files placed in the folder. This makes the new charts available to subsequent requests without having to restart the plugin (Feature request - Possible to re-read charts on the fly? #28).

  2. Adds support for processing mapbox style json files placed in the folder paths configured in the plugin settings, which will in-turn produce a chart metadata entry.

Removing the use of a shared path aligns better with multi-provider support.
Tile paths have a fully qualified path, remove use of relative path.
@panaaj panaaj added enhancement New feature or request feature labels Sep 27, 2024
@panaaj panaaj changed the title Mapbox Style support WIP: Mapbox Style support Sep 27, 2024
@panaaj panaaj requested review from tkurki and sbender9 October 2, 2024 06:46
@panaaj panaaj changed the title WIP: Mapbox Style support Add Mapbox Style support Oct 2, 2024
@panaaj panaaj changed the title Add Mapbox Style support Add File watcher and Mapbox Style support Nov 30, 2024
@panaaj
Copy link
Member Author

panaaj commented Nov 30, 2024

@tkurki Can you please review and if all is OK it's good to publish.

This code seemed to solve Freeboard-SK issue SignalK/freeboard-sk#211

CHANELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
/chart-styles/${mapboxstyle.json}

# when access token is defined
/chart-styles/${mapboxstyle.json}?access_token=${token}
Copy link
Member

Choose a reason for hiding this comment

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

This is local url, right? So why is there an access token needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

The Mapbox style json file is served locally but may reference resources that are remote which require an access token...
Example: satellite-v9.jon

{
  "version": 8,
  "name": "Mapbox Satellite",
  "metadata": {
    "mapbox:autocomposite": true,
    "mapbox:type": "default"
  },
  "sources": {
    "mapbox": {
      "type": "raster",
      "url": "mapbox://mapbox.satellite",    // remote resource
      "tileSize": 256
    }
  },
...

package.json Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
const refreshProviders = async () => {
const td = Date.now() - (lastWatchEvent as number)
app.debug(`last watch event time elapsed = ${td}`)
if (lastWatchEvent && td > 5000) {
Copy link
Member

Choose a reason for hiding this comment

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

Did I understand the logic correctly: if we notice a change in some chart data we reload the chart data the first time the data is needed after 5 seconds of the change?

Could we not just mark the data as stale and when it is next needed reload it? Or always reload when a change is noticed? Why the 5 second delay?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is due to the way FSWatcher produces events and the ability to distinguish when a:

  1. New file is created
  2. An exisiting file is replaced
  3. An existing file is renamed
  4. Multiple files are being added / deleted

There are multiple events produced during a simple file rename with no discernable metadata to indicate the actual operation taking place.... so the delay is to ensure the stream of events has ceased to indicate the process has completed.
Without it we would trigger multiple reloads while a rename, etc. is in process and throw exceptions.

README.md Outdated
@@ -2,7 +2,14 @@

Signal K Node server plugin to provide chart metadata, such as name, description and location of the actual chart tile data.

Supports both v1 and v2 Signal K resources api paths.
Chart metadata is derived from the following supported chart files:
- Mapbox Tiles _(.mbtiles)_
Copy link
Member

Choose a reason for hiding this comment

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

Afaik MBTiles is better known as just MBTiles and touted as open format. The spec README never mentions Mapbox, while it is under their Github org.

We could link to the canonical resources?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to MBTiles.
Which canonical resources are you refferring to?

src/charts.ts Outdated Show resolved Hide resolved
@tkurki
Copy link
Member

tkurki commented Jan 5, 2025

Unrelated to this PR: README is not very consistent

  • default location repeated
  • the steps-oriented configuration instructions are not very universal: some people will use online charts, others mbtiles etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants