airspy / airspyone_host

AirSpy's usemode driver and associated tools
http://airspy.com
247 stars 88 forks source link

fix parameter value check for airspy_set_linearity and airspy_set_sen… #53

Closed TLeconte closed 6 years ago

TLeconte commented 6 years ago

fix issue #52

touil commented 6 years ago

Are you aware of this? https://github.com/airspy/airspyone_firmware/blob/master/airspy_m0/airspy_m0.c#L162 https://github.com/airspy/airspyone_firmware/blob/master/airspy_m0/airspy_m0.c#L166

This is not an RTL dongle. The IF bandwidth is already matched to the sample rate in the firmware.

TLeconte commented 6 years ago

hum sorry, I did not to ask for merge about this set_bandwidth soon. I just wanted to commit it on my fork .... my bad,

To answer your question , hopefully you set the If bandwidth according to the sample rate, but there are cases where man want narrower bandwidth than half the sample rate. Particularly as the airspy don't allow to much choice about sample rate.

In fact, my set_bandwidth code is just a direct port of airspy linrad set bandwidth code. I wanted to measure and improve it before asking for merging ...

touil commented 6 years ago

Linrad's R820T IF code is itself based on our firmware code.

TLeconte commented 6 years ago

But add the possibility to change bandwidth down to 0.5Mhz See https://sourceforge.net/p/linrad/code/HEAD/tree/trunk/airspy.c lines : 380 , 403 and 967

touil commented 6 years ago

So can SDR#, SDR-Console, etc. There's a good reasons why airspy_r820t_write() exists. First, the IF filter is not symmetrical, which requires extra DSP processing (frequency translation + decimation) and retuning to get IF centered again. That's too application specific to be put in the library. Yet, it's a bad idea expose a half-assed API that hides these details. Either you add the accompanying DSP and retune code, OR don't hide the details. A "bad abstraction" costs more in the long term than no functionality. Leif is a smart grandpa and knows about these details. That's why he added the app specific code in his app and not in the library. Nothing was done by chance.

TLeconte commented 6 years ago

As I said, the intent was not to ask for merging this commit . I know that the filter is not symmetric.I will not call set_bandwidth a half-assed API but I agree that it needs to do the job as expected which need more works. About Linrad, I did not see the retune part which exist in the librtlsdr (but I could miss it , I did not read all the code). And no, I don't program by chance .

touil commented 6 years ago

OK. No problem. In general we call APIs that do half the job "half assed APIs", or more politically correct, "bad abstractions". If you find a more elegant solution that satisfies the semantics of IF filtering, you are more than welcome to contribute it.

TLeconte commented 6 years ago

and what about my first commit that fix an erroneous bound check ?

touil commented 6 years ago

Ben will check and merge. There's still a lot of left-over from HackRF.

bvernoux commented 6 years ago

Could you rebase your code and merge with actual master as there is conflict (mainly with new fprintf(stderr ....) see minor stuff #54

TLeconte commented 6 years ago

ok, will redo a better pull request