shashwatak / satellite-js

Modular set of functions for SGP4 and SDP4 propagation of TLEs.
MIT License
902 stars 145 forks source link

Fix dopplerFactor #79

Closed Alesha72003 closed 3 years ago

Alesha72003 commented 3 years ago

This is my first pull request. I may be doing something wrong English is not my native language This commit fixes the computation of the Doppler effect. DopplerFactor now takes into account the observer's movement.

Before changes: before

After changes: after

Code used:

``` satellite = require("./dist/satellite.js"); // Sample TLE var tleLine1 = '1 25544U 98067A 20295.53986052 .00000736 00000-0 21295-4 0 9995', tleLine2 = '2 25544 51.6441 83.2669 0001485 51.6840 75.2474 15.49319033251600'; // Initialize a satellite record var satrec = satellite.twoline2satrec(tleLine1, tleLine2); var observerGd = { longitude: satellite.degreesToRadians(37.6155000), latitude: satellite.degreesToRadians(55.7522000), height: 0 }; function calc() { var positionAndVelocity = satellite.propagate(satrec, new Date()); // The position_velocity result is a key-value pair of ECI coordinates. // These are the base results from which all other coordinates are derived. var positionEci = positionAndVelocity.position, velocityEci = positionAndVelocity.velocity; var gmst = satellite.gstime(new Date()); // You can get ECF, Geodetic, Look Angles, and Doppler Factor. var positionEcf = satellite.eciToEcf(positionEci, gmst), observerEcf = satellite.geodeticToEcf(observerGd), observerEci = satellite.ecfToEci(observerEcf, gmst), positionGd = satellite.eciToGeodetic(positionEci, gmst), lookAngles = satellite.ecfToLookAngles(observerGd, positionEcf), dopplerFactorEci = satellite.dopplerFactor(observerEci, positionEci, velocityEci); // Look Angles may be accessed by `azimuth`, `elevation`, `range_sat` properties. var azimuth = satellite.radiansToDegrees(lookAngles.azimuth), elevation = satellite.radiansToDegrees(lookAngles.elevation), rangeSat = lookAngles.rangeSat; console.log("Date: " + new Date().toISOString(), "azimuth: " + Math.round(azimuth*100)/100, "elevation: " + Math.round(elevation*100)/100, "Doppler for 100 MHz: " + Math.round(100 * dopplerFactorEci * 1e6) / 1e6 ); } setInterval(calc, 100); ```
coveralls commented 3 years ago

Coverage Status

Coverage increased (+0.8%) to 82.662% when pulling 638c02f2974cfcec5d57f670a59a1e8768a232f5 on Alesha72003:correct-doppler-factor into e7aacfeff2a20e8a809a0552a26f78e288a23bc5 on shashwatak:develop.

Alesha72003 commented 3 years ago

I tried making tests but not sure if they are simple. 1) There is no observer movement (since it is on the Earth's axis of rotation), the satellite's velocity vector is perpendicular to the observer-satellite vector, therefore, in this test, the Doppler effect should not affect the frequency. This test verifies the basic principle of the Doppler effect 2) There is movement of the observer, but the observer's velocity vector is perpendicular to the observer-satellite vector, so this movement does not affect the Doppler effect. This test verifies the correctness of accounting for the observer's movement. 3) A special case. This test verifies the correctness of the calculation taking into account the speeds of the satellite and the observer.

ezze commented 3 years ago

@Alesha72003 I think that your tests are more than fine. First of all, I meant that we need to improve test coverage and having some tests for dopplerFactor is much better than nothing. If something is broken it will be easier to fix it. Any additional tests can be provided later if required.

The last thing I want to be fixed is to use toBeCloseTo instead of toEqual. After that I can merge this PR and publish the changes to npm.

Alesha72003 commented 3 years ago

what value of numDigits to use in tests?

ezze commented 3 years ago

Let's take 8 digits, I think it will be enough.

Alesha72003 commented 3 years ago

All right?

ezze commented 3 years ago

@Alesha72003 Great! Just published 4.1.3. Give it a try!