bitcraze / lighthouse-fpga

GNU Lesser General Public License v3.0
27 stars 21 forks source link

Hard-coded pulse time delta causes simulations to fail #7

Closed jonnew closed 4 years ago

jonnew commented 4 years ago

I see there is a filter for determining if it is worth identifying a pulse's underlying polyID based on the time of its corresponding envelope compared to the last envelope detection:

https://github.com/bitcraze/lighthouse-fpga/blob/57d4204003ac0037bc4eb6aa7b0ad6c36b5bd01b/src/main/scala/lighthouse/PulseIdentifier.scala#L82

This constraint seems never to be met, e.g. when I run the simulation with Salae data provided in the repo. I'm just wondering (1) what the purpose of this is and (2) if perhaps this value should be a constructor argument to the PulseIdenifier to deal with e.g. if a different clock rate is used?

Thanks for this excellent project.

ataffanel commented 4 years ago

The purpose of this test is to identify pulses that hits different sensors during the same sweep. Searching for poly ID is expensive since we are doing it by initializing the LFSR bank with the first pulse data and running it until we find the second pulse data so If the two pulses are very far away it is going to be slow and since they are not in the same sweep the probability they come from the same basestation is very low. So we only consider pulses that come in a single sweep for identification.

Of course the reason we do that is because we identify the basestation from two pulses on two sensors in the same sweep: this is required since we need at least twice the lfsr width in number of bits to identify the polynomial that has generated the bits and a single pulse is not wide enough as soon as we are more than a couple of meter away from the basestation.

So this is not designed to run with a single sensor which is why it does not work with the Salae data. Maybe the best would be to generate or acquire test signals with multiple sensors, ideally this would be part of a testbench.

If you want to run the design at different clock, this should definitely be parametrized. If the goal is only to handle different clock the best is to use the spinal time handling like I am doing here: https://github.com/bitcraze/lighthouse-fpga/blob/57d4204003ac0037bc4eb6aa7b0ad6c36b5bd01b/src/main/scala/lighthouse/Lighthouse.scala#L436

This way a parameter is not even needed :-) (the time can be passed as parameter as well of course ). The width of polyFinder.io.maxTick would also need to be parametrized and calculated (likely with some log2Up() ).

jonnew commented 4 years ago

Ahhh!!! It all makes sense now :). Very clever! Thank you for the clear explanation! I will let you decide how to close this issue since I feel this information might be very helpful for others exploring the code. Thanks again.

krichardsson commented 4 years ago

Hi @jonnew! It seems as you reopened the issue, was that intentional?

jonnew commented 4 years ago

No, sorry. Feel free to close!