[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Discuss-gnuradio] digital_correlate_access_code_bb
From: |
Nowlan, Sean |
Subject: |
Re: [Discuss-gnuradio] digital_correlate_access_code_bb |
Date: |
Sun, 15 Apr 2012 22:30:49 +0000 |
Sorry to bump this... It appears the set_access_code method only occurs once
from within the constructor code. I'm not arguing that the old method isn't
faster, but it's functionally imprecise and the overhead of the if-else
statement isn't huge over the life of the object instance. If a time-variable
access code scheme were implemented, I could see it adding up, though. But
set_access_code isn't even SWIGged up as a public method, so I assume there
hasn't been demand for such a use case...
Sean
-----Original Message-----
From: address@hidden [mailto:address@hidden On Behalf Of Tom Rondeau
Sent: Monday, April 09, 2012 5:23 PM
To: Nick Foster
Cc: address@hidden
Subject: Re: [Discuss-gnuradio] digital_correlate_access_code_bb
On Mon, Apr 9, 2012 at 2:02 PM, Nick Foster <address@hidden> wrote:
> On Mon, Apr 9, 2012 at 10:48 AM, Marcus D. Leech <address@hidden> wrote:
>>
>> On 04/09/2012 01:38 PM, Tom Rondeau wrote:
>>>
>>> On Sat, Apr 7, 2012 at 10:12 PM, Marcus D. Leech<address@hidden>
>>> wrote:
>>>>
>>>> Just looking at this function:
>>>>
>>>> correlate_access_code_bb
>>>>
>>>> In the method set_access_code, it takes a string. Which should be
>>>> ASCII '1'
>>>> and '0' characters to represent the binary sequence being
>>>> correlated against.
>>>>
>>>> Here's a little beauty of a code snippet:
>>>>
>>>> d_access_code = 0;
>>>> for (unsigned i=0; i< 64; i++){
>>>> d_access_code<<= 1;
>>>> if (i< len)
>>>> d_access_code |= access_code[i]& 1; // look at LSB only
>>>> }
>>>>
>>>> This relies on the fact that ASCII '1' and '0' happen to have
>>>> low-order bits of the right "flavour". This is insanely dirty and
>>>> gross and I can't
>>>> believe we let this nonsense in the code base.
>>>>
>>>> There's no reason not to do the right thing here.
>>>>
>>>>
>>>> --
>>>> Marcus Leech
>>>> Principal Investigator
>>>> Shirleys Bay Radio Astronomy Consortium http://www.sbrac.org
>>>
>>>
>>> Want to submit a patch?
>>>
>>> Tom
>>>
>>>
>> Attached.
>
>
> While you're patching correlate_access_code_bb, please patch
> correlate_access_code_tag_bb with the attached patch.
>
> --n
So my guess is that the use of the binary & operator is to avoid the need for
an if/if else/else branching check. It was most likely done for efficiency. So
while this patch might be the "right" way to do it from a code perspective, it
could result in slower code (on certain machines that don't handle branch
prediction well). It does make assumptions about the correctness of the access
code, though.
Tom
_______________________________________________
Discuss-gnuradio mailing list
address@hidden
https://lists.gnu.org/mailman/listinfo/discuss-gnuradio
- [Discuss-gnuradio] digital_correlate_access_code_bb, Marcus D. Leech, 2012/04/07
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Thomas Tsou, 2012/04/07
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Tom Rondeau, 2012/04/09
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Marcus D. Leech, 2012/04/09
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Nick Foster, 2012/04/09
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Tom Rondeau, 2012/04/09
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb,
Nowlan, Sean <=
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Tom Rondeau, 2012/04/19
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Nowlan, Sean, 2012/04/19
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Tom Rondeau, 2012/04/23
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Nowlan, Sean, 2012/04/23
- Re: [Discuss-gnuradio] digital_correlate_access_code_bb, Tom Rondeau, 2012/04/23