debrouxl / tilibs

TILP (formerly GtkTiLink) can transfer data between Texas Instruments graphing calculators and a computer. It works with all link cables (parallel, serial, Black/Gray/Silver/Direct Link) and it supports the TI-Z80 series (73..86), the TI-eZ80 series (83PCE, 84+CE), the TI-68k series (89, 92, 92+, V200, 89T) and the Nspire series (Nspire Clickpad / Touchpad / CX, both CAS and non-CAS)
http://lpg.ticalc.org/prj_tilp
67 stars 22 forks source link

libticables: add cmake check for parallel and serial includes #60

Closed davide125 closed 2 years ago

davide125 commented 2 years ago

Check for the includes for parallel and serial support and set the relevant defines, so that support can be compiled in when available.

debrouxl commented 2 years ago

The changes look sound. @adriweb ?

adriweb commented 2 years ago

Looks good to me but I'll check on macOS quickly.

adriweb commented 2 years ago

Well, it fails to build now for me:

[ 96%] Linking CXX shared library libticables2.dylib
undef: _cable_par
Undefined symbols for architecture x86_64:
  "_cable_par

We can see the usage here in ticables.cc:

#if !defined(NO_CABLE_PAR) && (defined(HAVE_LINUX_PARPORT_H) || defined(__WIN32__))
    &cable_par,
#endif

Looking at the changes a bit more, it's because defining a macro with no value still makes it defined, so the check in the code ends up not doing what we want.

So, if we don't want to change the code (to check for an actual value, like 1 which cmake uses, not sure about the autotools), only the CMakeLists.txt, we should do that:

if (HAVE_LINUX_PARPORT_H)
    target_compile_definitions(ticables2_objlib PUBLIC HAVE_LINUX_PARPORT_H=1)
endif()
if (HAVE_LINUX_SERIAL_H)
    target_compile_definitions(ticables2_objlib PUBLIC HAVE_LINUX_SERIAL_H=1)
endif()
davide125 commented 2 years ago

Thanks! I forgot cmake doesn't like undefined variables. I've retested on my end and confirmed this still does the right thing on Linux.

adriweb commented 2 years ago

No problem, I'm not a CMake expert anyway (maybe there's a shorter/cleaner way to do all that), so I wouldn't have caught it if I hadn't tried :P

debrouxl commented 2 years ago

Thanks :)