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

invalid(may) LTS correaltion coefficients in sync_long.v #8

Closed
littlelid opened this issue Mar 28, 2020 · 7 comments
Closed

invalid(may) LTS correaltion coefficients in sync_long.v #8

littlelid opened this issue Mar 28, 2020 · 7 comments

Comments

@littlelid
Copy link

In file sync_long.v line 418-421, the original code is
stage_X0 <= cross_corr_buf[1];
stage_X1 <= cross_corr_buf[2];
stage_X2 <= cross_corr_buf[3];
stage_X3 <= cross_corr_buf[4];

I think the code should be fixed with:
stage_X0 <= cross_corr_buf[0];
stage_X1 <= cross_corr_buf[1];
stage_X2 <= cross_corr_buf[2];
stage_X3 <= cross_corr_buf[3];

@mmehari
Copy link
Member

mmehari commented Mar 28, 2020

I remember looking at this when I was studying the rx core.
I also have tried the same modification you did and I haven't seen that much of a difference.
For the sake of leaving the originality of the code, I left it as it is but you are free to change it and please let us know if there is a difference in rx performance.

@littlelid
Copy link
Author

There might be one explanation.
The best practice to detect LTS is to use all 64 coefficients. To save computing resources, the rx core only uses 16 coefficients, and also works well.
If these four buffs have mistakes, the actual LTS detection can be modeled as using 12 coefficients to cross-correlate with base-band samples. At high SNR, the detection still works. But in low SNR, I believe, the performance might decrease.

@mmehari
Copy link
Member

mmehari commented Mar 28, 2020

What you said is true as the author reduced the size of the cross-validation to 16 IQ samples (i.e. https://openofdm.readthedocs.io/en/latest/sync_long.html#fig-match-size).

And it might be the case that I tested the changes (don't remember exactly) at high SNR which hasn't made any significant change.

@JiaoXianjun
Copy link
Member

Thanks for reporting. Did you ever try low SNR such as by longer distance?

Anyway we will fix and test in low SNR.

@JiaoXianjun
Copy link
Member

I have updated all stuffs accordingly. Issue is closed.

@JiaoXianjun
Copy link
Member

I have done a test between Thinkpad T420 (Intel wifi card: Intel Corporation Centrino Advanced-N 6205) and zcu102+fmcs2. openwifi as AP, T420 as client.

The results shows that original sync_long.v index has better performance clearly. We need to figure out further. But for now, let's use the original index.

Original index:
stage_X0 <= cross_corr_buf[1]; stage_X1 <= cross_corr_buf[2]; stage_X2 <= cross_corr_buf[3]; stage_X3 <= cross_corr_buf[4];

New index:
stage_X0 <= cross_corr_buf[0]; stage_X1 <= cross_corr_buf[1]; stage_X2 <= cross_corr_buf[2]; stage_X3 <= cross_corr_buf[3];
Iperf TCP performance of original index:
DL: 27M; UL: 22M

Iperf TCP performance of new index:
DL: 18M; UL: 16M

JiaoXianjun added a commit to open-sdr/openofdm that referenced this issue Apr 17, 2020
JiaoXianjun added a commit that referenced this issue Apr 17, 2020
@JiaoXianjun
Copy link
Member

Hello,

How are you doing with openwifi now? Do you still have any issue? The openwifi has been improved a lot during the last .5 year. It supports more boards (high end to low end) and becomes more stable.

Would you please tell us your email (if you could also introduce yourself bit, that would be perfect). We might send out some questions to listen for user feed feedback.

Thanks.

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

No branches or pull requests

3 participants