librtlsdr icon indicating copy to clipboard operation
librtlsdr copied to clipboard

librtlsdr shouldn't output to standard error

Open jwpowell opened this issue 10 years ago • 5 comments

The library outputs to standard error via fprintf() calls throughout librtlsdr.c. For a library, this isn't best practice since it can interfere with command line programs. For example, I've run into this issue while prototyping using pyrtlsdr. There seems to be three possible solutions to this:

  • Remove all fprintf() calls in librtlsdr.c. This is the easiest solution, but it makes debugging programs more difficult without the error information.
  • Refactor the API so the error information is reflected via the API rather than via standard error. This is hard and will break the API.
  • Replace all fprintf() calls in librtlsdr.c with a function that only outputs to standard error when the API consumer turns on debug mode. This is a middle ground between the two above. It doesn't break the API, but it adds more kludge.

jwpowell avatar Jan 18 '15 16:01 jwpowell

How is stderr interfering with command line programs?

Hoernchen avatar Jan 18 '15 16:01 Hoernchen

jwpowell is right, libraries should never have side-effects like logging or printing unless otherwise instructed. Especially if there's no way to disable or filter it.

bemasher avatar Jan 19 '15 01:01 bemasher

@Hoernchen, as @bemasher pointed out, side-effects in libraries aren't great. As an example, imagine an ncurses interface with mixers and tuners that plays FM broadcast radio. The fprintf() calls would trash that. Personally, I've thought about creating a sort of CLI shell that you can explore the spectrum with. Again, the library's debugging output would dirty that.

jwpowell/librtlsdr@23b0fb7a603ba11d8437b1565490772c133eecc4

This switches debug according to the standard NDEBUG preprocessor macro. This switches it off at compile-time. Extra work might be put into this to have it switchable at run-time. I haven't used this commit in real-world testing, so I don't think it's ready for a pull request. Comments welcome.

jwpowell avatar May 02 '15 17:05 jwpowell

what about a callback the caller sets, together with an error level through a new librtlsdr function? prototype could be: typedef void (* rtlsdr_error_callback)( int errLevel, int errCode, const char* errText );

with errLevel: 0 == fatal error, 1 = error, 2 = warning, 3 = .. errCode: some specific error identifier, and errText a zero terminated C string, prepared with snprintf().

then having a few global variables inside librtlsdr: rtlsdr_error_callback * rtl_err_callback = 0; int rtlsdr_err_level_thresh = 2; /* callback only for warnings and higher priority (lower level value) msgs */

hayguen avatar Jun 18 '15 20:06 hayguen

+1 would be good to keep stderr for errors :)

Romeo-Golf avatar Dec 22 '16 23:12 Romeo-Golf