Open quentinmit opened 5 years ago
General early comments:
Could you explain why nodeIdCompat
is necessary? I would rather not give up using actual URLs and cram everything including tests into the module-ID model.
Since geodesy is a dependency, there isn't much value specifically in computing the modified module dynamically. How about doing the edits inside fetch-js-deps.sh
instead, which would be less complication to the web code?
"The server really needs to track the selected record" — indeed it does, particularly for satellites. Given the amount of fiddling needed to make this work (which, I admit, is possibly useful for other features), I'm tempted to suggest holding off on this shortest-path-to-feature and instead get that infrastructure piece in. Maybe I should work on that.
General early comments:
- Could you explain why
nodeIdCompat
is necessary? I would rather not give up using actual URLs and cram everything including tests into the module-ID model.
I tried to cover this in the commit messages, but the short answer is, latlon-spherical.js
contains this line:
var Dms = require('./dms.js');
and RequireJS misinterprets that as relative to the HTML file and not the javascript file that includes it.
I think I could also make it work by using RequireJS's "shims" mechanism, which would save the transformation step but pollute the global namespace with LatLon
and Dms
objects and would need to be added to every place that calls requirejs.config
; would that be better?
- Since geodesy is a dependency, there isn't much value specifically in computing the modified module dynamically. How about doing the edits inside
fetch-js-deps.sh
instead, which would be less complication to the web code?
If we did anything like that we'd have to give up on geodesy being a submodule. If you want to do that I'd rather just fork it and ship the forked version with shinysdr.
- "The server really needs to track the selected record" — indeed it does, particularly for satellites. Given the amount of fiddling needed to make this work (which, I admit, is possibly useful for other features), I'm tempted to suggest holding off on this shortest-path-to-feature and instead get that infrastructure piece in. Maybe I should work on that.
I thought about that, but on balance I went ahead with a client-side solution because it's more than enough for use with database entries, and moving targets is an edge-case.
You can look at https://github.com/w1xm/shinysdr/tree/rotator-tune-shim to see what this would look like using shim
instead of nodeIdCompat
.
-Suggestion: web ui should make a distinct visual indication between a tracking plot and Antenna position. Focus on code to update the predicted position and plot that on the globe.
Comment: importing antenna beamwidth/focal length of station properties. This effect allows directional reception rather than precise tracking. important when pairing directional dish aperture with tracking speed, and reporting this all to the web interface.
Q; Show the current predicted position ? or Current predicted position along a path? Icon moves along the ArcPath? Resulting in a checksum that returns plotting lock on for position alignment for the page.
Just to set expectations, I don't expect to get around to further action on this PR until January. This is partly because of the holidays, and partly because the module loading awkwardness is tempting me to implement the """correct""" solution here (server-side continuous tracking) instead and so I may give that a shot.
This required some interesting surgery to work around RequireJS bugs when loading the geodesy library.
geodesy is built as a CommonJS library, which RequireJS provides support for but it is partially broken. More detail is in the commit messages.
I could have instead patched the geodesy source or used one of the existing module converters out there, but I didn't want to add any additional build steps and build-time dependencies.