-
Notifications
You must be signed in to change notification settings - Fork 131
refactor: migrate sensors from TypeScript to JSON data #1258
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: develop
Are you sure you want to change the base?
refactor: migrate sensors from TypeScript to JSON data #1258
Conversation
- Create sensors.json with all sensor data extracted from sensors.ts - Add sensorLoader.ts to load sensors from JSON and instantiate sensor objects - Update all imports to use sensorLoader instead of sensors - Add conversion script (parse-sensors.mjs) to regenerate JSON from TypeScript - Add documentation (README.md) explaining the new architecture This change enables sensors to be loaded from remote sources in the future without requiring code changes, while maintaining full type safety. Files modified: - src/app/data/catalog-manager.ts - src/app/sensors/sensorManager.ts - src/plugins/sensor-list/sensor-list.ts - src/plugins/sensor/multi-site-look-angles-plugin.ts - test/environment/apiMocks.ts Files added: - src/app/data/catalogs/sensors.json (77 sensors) - src/app/data/catalogs/sensorLoader.ts - src/app/data/catalogs/README.md - scripts/parse-sensors.mjs
|
|
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.
Pull Request Overview
This PR refactors the sensor data architecture by migrating from hardcoded TypeScript sensor definitions to a JSON-based data format. This change enables future remote data loading without requiring code modifications while maintaining type safety through runtime conversion.
Key Changes
- Extracted 77 sensor definitions from TypeScript to JSON format
- Created a loader module that instantiates typed sensor objects from JSON data
- Updated all sensor imports across the codebase to use the new loader
- Added a conversion script to regenerate JSON from TypeScript if needed
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/data/catalogs/sensors.json | New file containing all 77 sensor configurations in JSON format with enum values as strings |
| src/app/data/catalogs/sensorLoader.ts | Loader module that parses JSON, converts enum strings to values, and instantiates sensor objects |
| src/app/data/catalogs/README.md | Documentation explaining the new architecture and migration workflows |
| scripts/parse-sensors.mjs | Node.js script to regenerate JSON from TypeScript sensor definitions using regex parsing |
| src/app/data/catalog-manager.ts | Updated import to use sensorLoader instead of sensors |
| src/app/sensors/sensorManager.ts | Updated import to use sensorLoader instead of sensors |
| src/plugins/sensor-list/sensor-list.ts | Updated import to use sensorLoader instead of sensors |
| src/plugins/sensor/multi-site-look-angles-plugin.ts | Updated import to use sensorLoader instead of sensors |
| test/environment/apiMocks.ts | Updated import to use sensorLoader instead of sensors |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sensor[key] = content.split(',').map(item => { | ||
| const trimmed = item.trim(); | ||
|
|
||
| return isNaN(Number(trimmed)) ? trimmed : parseFloat(trimmed); | ||
| }); |
Copilot
AI
Nov 18, 2025
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.
The parsing logic at line 142-146 has inconsistent handling of array elements. When JSON.parse fails on a cleaned array, the code tries to split by comma and parse individual items, but it doesn't handle quoted strings within the array properly. This could lead to incorrect parsing if array elements contain commas within quoted strings.
| // Ensure typed values are properly cast | ||
| if (sensorConfig.lat !== undefined) { | ||
| sensorConfig.lat = sensorConfig.lat as Degrees; | ||
| } | ||
| if (sensorConfig.lon !== undefined) { | ||
| sensorConfig.lon = sensorConfig.lon as Degrees; | ||
| } | ||
| if (sensorConfig.alt !== undefined) { | ||
| sensorConfig.alt = sensorConfig.alt as Kilometers; | ||
| } | ||
| if (sensorConfig.minAz !== undefined) { | ||
| sensorConfig.minAz = sensorConfig.minAz as Degrees; | ||
| } | ||
| if (sensorConfig.maxAz !== undefined) { | ||
| sensorConfig.maxAz = sensorConfig.maxAz as Degrees; | ||
| } | ||
| if (sensorConfig.minEl !== undefined) { | ||
| sensorConfig.minEl = sensorConfig.minEl as Degrees; | ||
| } | ||
| if (sensorConfig.maxEl !== undefined) { | ||
| sensorConfig.maxEl = sensorConfig.maxEl as Degrees; | ||
| } | ||
| if (sensorConfig.minRng !== undefined) { | ||
| sensorConfig.minRng = sensorConfig.minRng as Kilometers; | ||
| } | ||
| if (sensorConfig.maxRng !== undefined) { | ||
| sensorConfig.maxRng = sensorConfig.maxRng as Kilometers; | ||
| } | ||
| if (sensorConfig.changeObjectInterval !== undefined) { | ||
| sensorConfig.changeObjectInterval = sensorConfig.changeObjectInterval as Milliseconds; | ||
| } | ||
| if (sensorConfig.beamwidth !== undefined) { | ||
| sensorConfig.beamwidth = sensorConfig.beamwidth as Degrees; | ||
| } | ||
| if (sensorConfig.boresightAz && Array.isArray(sensorConfig.boresightAz)) { | ||
| sensorConfig.boresightAz = sensorConfig.boresightAz.map((v: number) => v as Degrees); | ||
| } | ||
| if (sensorConfig.boresightEl && Array.isArray(sensorConfig.boresightEl)) { | ||
| sensorConfig.boresightEl = sensorConfig.boresightEl.map((v: number) => v as Degrees); | ||
| } | ||
|
|
Copilot
AI
Nov 18, 2025
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.
The parsing logic doesn't handle the additional sensor range/azimuth/elevation fields (minAz2, maxAz2, minEl2, maxEl2, minRng2, maxRng2) that exist in the JSON data. Several sensors use these fields (e.g., COBRADANE at lines 280-285, LEOMSR at lines 853-858, LEOCRSR at lines 934-939, LEOAZORES at lines 965-970, NRC at lines 1094-1099). These numeric values should be properly typed as Degrees or Kilometers like their primary counterparts.
| "id": 0, | ||
| "name": "Pechora, Russia", | ||
| "uiName": "Pechora", | ||
| "system": "Dar\\'yal", |
Copilot
AI
Nov 18, 2025
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.
Typo in sensor name: "Dar\'yal" should be "Dar'yal". The escaped backslash in the JSON string will result in the display showing a literal backslash instead of just an apostrophe.
| "system": "Dar\\'yal", | |
| "system": "Dar'yal", |
| "BLEAFB": { | ||
| "_sensorType": "RfSensor", | ||
| "objName": "BLEAFB", | ||
| "shortName": "BLE", | ||
| "id": 0, | ||
| "name": "Beale AFB, California", | ||
| "uiName": "Beale AFB", | ||
| "system": "PAVE PAWS UEWR", | ||
| "freqBand": "UHF", | ||
| "type": "SpaceObjectType.PHASED_ARRAY_RADAR", | ||
| "lat": 39.136064, | ||
| "lon": -121.351237, | ||
| "alt": 0.112, | ||
| "minAz": 126, | ||
| "maxAz": 6, | ||
| "boresightAz": [ | ||
| 186, | ||
| 306 | ||
| ], | ||
| "minEl": 3, | ||
| "maxEl": 85, | ||
| "boresightEl": [ | ||
| 20, | ||
| 20 | ||
| ], | ||
| "minRng": 200, | ||
| "maxRng": 5556, | ||
| "changeObjectInterval": 1000, | ||
| "beamwidth": 2, | ||
| "commLinks": [ | ||
| "CommLink.AEHF", | ||
| "CommLink.WGS" | ||
| ], | ||
| "zoom": "ZoomValue.LEO", | ||
| "country": "United States", | ||
| "operator": "Operators.USSF" | ||
| }, |
Copilot
AI
Nov 18, 2025
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.
The BLEAFB sensor is missing the sensorId field (line 77) which is present in the CODSFS sensor (line 39). This inconsistency could lead to issues if the sensorId field is required for sensor identification or processing.
|
|
||
| // Remove comments at the end (but only after quoted strings or other values) | ||
| // This regex looks for comments that start after a quote, comma, or closing bracket | ||
| value = value.replace(/(['"\],}])\s*\/\/.*$/, '$1'); |
Copilot
AI
Nov 18, 2025
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.
The regex pattern for removing comments (value.replace(/(['"\],}])\s*\/\/.*$/, '$1')) at line 106 is too restrictive and may not catch all inline comments. For example, it won't remove comments that appear after numeric values or unquoted identifiers (e.g., maxEl: 90 // comment). Consider using a more comprehensive pattern like /\s*\/\/.*$/ after proper string/array parsing.
| value = value.replace(/(['"\],}])\s*\/\/.*$/, '$1'); | |
| value = value.replace(/\s*\/\/.*$/, ''); |
| sensor[key] = value; | ||
| } | ||
| } catch (e) { | ||
| // If parsing fails, store as string |
Copilot
AI
Nov 18, 2025
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.
[nitpick] The fallback logic for failed parsing wraps values in a try-catch (line 120) but doesn't log or handle the error. When JSON parsing or other conversions fail, the code silently stores the value as a string, which could hide data format issues. Consider logging warnings when parsing fails to help debug potential issues during sensor data conversion.
| // If parsing fails, store as string | |
| // If parsing fails, store as string and log a warning | |
| console.warn(`Warning: Failed to parse value for key "${key}". Storing as string. Value: ${JSON.stringify(value)}. Error: ${e.message}`); |
|
|
||
| - **sensors.json**: Contains all sensor data in JSON format | ||
| - **sensorLoader.ts**: Loads sensors from JSON and creates sensor objects | ||
| - **sensors.ts**: Legacy TypeScript file (kept for type definitions and enums) |
Copilot
AI
Nov 18, 2025
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.
Documentation states that sensors.ts is a "Legacy TypeScript file (kept for type definitions and enums)" at line 11, but the PR description indicates this is a migration away from TypeScript. It's unclear whether sensors.ts is being kept in the repository or removed. If it's being kept, this should be clarified. If it's being removed, the documentation should be updated to reflect that type definitions are now in sensorLoader.ts.
| const response = await fetch('https://api.example.com/sensors.json'); | ||
| const sensorsData = await response.json(); | ||
| ``` | ||
|
|
Copilot
AI
Nov 18, 2025
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.
The async/await example code for remote loading (lines 58-60) is misleading because the current implementation in sensorLoader.ts uses a synchronous import, not an async fetch. If the intention is to support remote loading, the loadSensorsFromJson() function would need to be made async and all calling code would need to be updated. Consider clarifying that this is a future enhancement or providing a more complete example of the necessary changes.
| **Note:** The current implementation in `sensorLoader.ts` loads sensors synchronously from a local JSON file. To support remote loading as shown above, you would need to make the loader function (`loadSensorsFromJson()`) asynchronous and update all calling code to use `await` or handle Promises. This is a future enhancement and not currently supported. |
| 'SpaceObjectType.PHASED_ARRAY_RADAR': SpaceObjectType.PHASED_ARRAY_RADAR, | ||
| 'SpaceObjectType.OPTICAL': SpaceObjectType.OPTICAL, | ||
| 'SpaceObjectType.MECHANICAL': SpaceObjectType.MECHANICAL, | ||
| 'SpaceObjectType.OBSERVER': SpaceObjectType.OBSERVER, |
Copilot
AI
Nov 18, 2025
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.
Missing SpaceObjectType.BISTATIC_RADIO_TELESCOPE in the enum mapping. The sensors.json file contains sensors with type "SpaceObjectType.BISTATIC_RADIO_TELESCOPE" (e.g., NRC, Tromso, Kiruna, Sodankyla, Svalbard sensors at lines 1084, 1229, 1253, 1277, 1301), but this enum value is not included in the SpaceObjectTypeMap. This will cause these sensors to have undefined as their type instead of the proper enum value.
| 'SpaceObjectType.OBSERVER': SpaceObjectType.OBSERVER, | |
| 'SpaceObjectType.OBSERVER': SpaceObjectType.OBSERVER, | |
| 'SpaceObjectType.BISTATIC_RADIO_TELESCOPE': SpaceObjectType.BISTATIC_RADIO_TELESCOPE, |
| "name": "Lin\\'an Zhejiang, China", | ||
| "uiName": "Lin\\'an", |
Copilot
AI
Nov 18, 2025
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.
Typo in sensor name: "Lin\'an" should be "Lin'an". The escaped backslash in the JSON string at line 1666 will result in the display showing a literal backslash instead of just an apostrophe. This also appears at line 1369 in "Dar\'yal".
| "name": "Lin\\'an Zhejiang, China", | |
| "uiName": "Lin\\'an", | |
| "name": "Lin'an Zhejiang, China", | |
| "uiName": "Lin'an", |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const SpaceObjectTypeMap: Record<string, SpaceObjectType> = { | ||
| 'SpaceObjectType.PHASED_ARRAY_RADAR': SpaceObjectType.PHASED_ARRAY_RADAR, | ||
| 'SpaceObjectType.OPTICAL': SpaceObjectType.OPTICAL, | ||
| 'SpaceObjectType.MECHANICAL': SpaceObjectType.MECHANICAL, | ||
| 'SpaceObjectType.OBSERVER': SpaceObjectType.OBSERVER, |
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.
Map bistatic sensor types to SpaceObjectType
The new loader only maps four sensor type strings back to the SpaceObjectType enum. Sensors that are tagged as "SpaceObjectType.BISTATIC_RADIO_TELESCOPE" in the JSON (e.g., NRC at sensors.json line 1084) are left as raw strings, so any checks against SpaceObjectType.BISTATIC_RADIO_TELESCOPE will now fail and those sensors will be treated as an unknown type at runtime. Please add this enum value to the map so these sensors deserialize correctly.
Useful? React with 👍 / 👎.
|




This change enables sensors to be loaded from remote sources in the future without requiring code changes, while maintaining full type safety.
Files modified:
Files added: