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

Rearrange phase enhancement so that it handles phase errors in both directions #36

Merged

Conversation

mutability
Copy link

The idea here is to look at the known bits of the preamble and see in which direction they have leaked by looking at the amplitude of adjacent samples. This gives us the direction of the phase error between the signal and our sampling period, and a rough idea of the size of the error.

If we see energy in later-than-expected samples, our sample period must start early and include some of the previous bit period. The existing enhancement algorithm handles this, so this is basically unchanged except that the scaling factor is weighted by the size of the phase error.

If we see energy in earlier-than-expected samples, our sample period must start late and include some of the next bit period. So we need to do the same thing but in the opposite direction - each sample pair influences the previous sample, so work backwards through the data applying that.

In my tests this almost doubles the number of messages recovered by phase enhancement (which is consistent with the existing approach only handling one direction of phase shift)

…irections.

This almost doubles the number of messages recovered by phase enhancement.
@MalcolmRobb
Copy link
Owner

I like what you're trying to do, but It's not safe as it is. Basically, pPreamble[-1] is unsafe because in the first iteration through the loop, pPreamble = &m[j], where m = Modes.magnitude and j = zero. Therefore, you're using a value that is in memory prior to the start of the array, and at best the value is invalid, at worst it causes a fatal protection fault.

The simple resolution is to not do the correction on the first iteration (j=1) but that's a bit messy really. There's got to be a cleaner resolution, but I can't see it at the moment.

I'll have a play with your routine and see if/how much it improves things though.

@mutability
Copy link
Author

J must be greater than zero when use correction is 1 so I don't think there is an illegal access here. The code does just what you suggest - doesn't correct at the first sample.

@MalcolmRobb
Copy link
Owner

Don't think so. The end of the loop where correction is enabled does this...
// Retry with phase correction if enabled, necessary and possible.
if (Modes.phase_enhance && !mm.crcok && !mm.correctedbits && !use_correction && j && detectOutOfPhase(pPreamble)) {
use_correction = 1; j--;
} else {
use_correction = 0;
}

Note the j-- on the end of the use_correction = 1. Therefore on the first loop through, j changes from 0 to -1, and then the for loop re-increments it to 0. The error correction will then access preamble[-1]

I think a better solution would probably be to look at the falling edge of bits 2/3 ,and the rising edge of bit 6/7 (so ignoring -1/0 and 9/10)

@mutability
Copy link
Author

Look at the condition in the if-statement you quoted; use_correction is only set to 1 if j is non-zero.

@MalcolmRobb
Copy link
Owner

Ahh, good point. Dunces cap on for the day.

@mutability
Copy link
Author

(from #50 - I assume that comment was meant from here)

Just so I'm clear - I'm talking about your lines 1487 and 1663 being unsafe.

1487 is part of applyPhaseCorrection, which assumes that pPayload[-1] is accessible - see the comment in the header. pPayload is passed from the call point at line 1664 which always passes &aux[1], so this is safe - aux[0] is always accessible.

1663 is:

memcpy(aux, &pPreamble[-1], sizeof(aux));

where pPreamble = &m[j]. This code can only run when j > 0, because the end-of-loop condition can only set use_correction = 1 when j > 0, so this is also safe.

@mutability
Copy link
Author

No worries, that code is a bit of a spiders web :)

@MalcolmRobb
Copy link
Owner

Yes - posting in the wrong thread, and I'm already wearing the dunces cap.

I now accept it's safe. Gotta be a nicer way of doing it on every loop though. I'll have a play with your code tonight.

MalcolmRobb added a commit that referenced this pull request Oct 31, 2014
Rearrange phase enhancement so that it handles phase errors in both directions
@MalcolmRobb MalcolmRobb merged commit bff92c4 into MalcolmRobb:master Oct 31, 2014
@MalcolmRobb
Copy link
Owner

I had a play with this last night. Observations are as follows.

The original code typically recovers around 5% extra frames using my test file.
Your new code typically recovers an extra 10% extra frames using my test file.

I have therefore merged the pull since it does extract more data. However, I have some observations.

The vast majority of the extra frames are DF11's. The problem with DF11's is that they aren't fully protected by CRC due to the overlaying of SI/II information. It is therefore difficult to be sure that all these received DF11's are actually completely valid.

The code does check that the DF11 comes from a known ICAO, so I there is little doubt that the frame is 'real'. The issue surrounds the decoding of the II/SI. This information is important for techniques like beamfinder (in Plane Plotter) to work reliably. As it stands, there is/are significant amounts of spurious II/SI decoding, and it's worse with weak or phase corrected data.

So a TODO has to be to investigate ways of improving the reliability of DF11 decoding when II/SI is used. In the mean time, using phase correction with plane plotter is likely to produce a greater number of spurious plots.

@mutability
Copy link
Author

Perhaps include a confidence value with each packet; the decoder could refuse to decode (or flag as suspect) messages that have low confidence and are not fully covered by CRC. Anything phase-corrected would be low confidence. You could either assume non-phase-corrected is high confidence always, or look for "almost ambiguous" bits where there is not much separation between the two levels of the manchester encoding.

@mutability mutability deleted the improve-phase-enhancement branch October 31, 2014 17:44
@MalcolmRobb
Copy link
Owner

Mutability,
I don't think all phase corrected samples need to be marked 'suspect'

My current thinking is to calculate a statistical probability that the message is 'valid' I'm thinking to do a standard deviation calculation on all the bits of the message. Each bit should either be a 1-0 or a 0-1, so calculate the difference between the 0 and 1 levels, and then do the standard deviation over all the bits in the message.

If the resultant sigma figure is low, that implies that all the bits have similar strength, and so should all be similarly valid. However, if the sigma shows large variance, then presumably there is some interference (overlapping Mode A/C/S signals) which is potentially corrupting the frame. For non CRC protected DF-11's this sigma level could be used to decide if the frame is "suspect" or not. That way, there is no need to distincuish between phase corrected and non phase corrected -I hope.

If a way can be found of getting well filtered reliable DF-11's with SI/II then the next step is to implement auto beam-finding on non position reporting aircraft. I've been playing with fourier transforms for over a year, but it's always scuppered by excessive 'random' II/SI frames.

Exadios pushed a commit to Narrogin-Gliding-Club/dump1090 that referenced this pull request Dec 3, 2019
Fix getting LAT/LON from piaware .env file
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.

2 participants