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

Fix dopplerFactor #79

Merged
merged 6 commits into from
Dec 18, 2020
Merged

Conversation

Alesha72003
Copy link
Contributor

@Alesha72003 Alesha72003 commented Oct 22, 2020

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
Copy link

coveralls commented Oct 22, 2020

Coverage Status

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

Copy link
Collaborator

@ezze ezze left a comment

Choose a reason for hiding this comment

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

@Alesha72003 @thkruz @shashwatak dopplerFactor function is not covered by any tests. Could you write a couple of simple tests for it in order to not break it in the future?

@Alesha72003
Copy link
Contributor Author

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
Copy link
Collaborator

ezze commented Dec 18, 2020

@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
Copy link
Contributor Author

what value of numDigits to use in tests?

@ezze
Copy link
Collaborator

ezze commented Dec 18, 2020

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

@Alesha72003
Copy link
Contributor Author

All right?

test/dopplerFactor.test.js Outdated Show resolved Hide resolved
@ezze ezze merged commit b177412 into shashwatak:develop Dec 18, 2020
@Alesha72003 Alesha72003 deleted the correct-doppler-factor branch December 18, 2020 14:23
@ezze
Copy link
Collaborator

ezze commented Dec 18, 2020

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

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

Successfully merging this pull request may close these issues.

4 participants