goToMain / libosdp

Implementation of IEC 60839-11-5 OSDP (Open Supervised Device Protocol); provides a C library with support for C++, Rust and Python3
https://libosdp.sidcha.dev
Apache License 2.0
134 stars 71 forks source link

libosdp as nested CMake project + static CP build #59

Closed schmida2 closed 3 years ago

schmida2 commented 3 years ago

I would like to use libosdp (static) nested into another CMake project, thus the libosdp project should refer to the folder of the CMakeLists.txt containing the most recent project() command. This is done by using PROJECT_SOURCE_DIR instead of CMAKE_SOURCE_DIR (See https://stackoverflow.com/questions/32028667/are-cmake-source-dir-and-project-source-dir-the-same-in-cmake)

Also, in my case the CP functionality of libosdp should go into another shared library, that is why I would like to add static building of library without the samples and tools (CONFIG_OSDP_STATIC_CP, similar to CONFIG_OSDP_STATIC_PD).

sidcha commented 3 years ago

@schmida2, your PROJECT_SOURCE_DIR changes and optional python dependancy looks good to me. I cannot understand the need for the static CP change.. You need to elaborate on the use case a bit.

The --static-pd option (which should have been called single PD mode) was added to build a version of this library that has no dynamic memory allocation whatsoever. This feature was requested by a user who is using this library in a very low memory embedded device which they are using to setup a singular PD (see how CONFIG_OSDP_STATIC_PD is used in code to achieve this). The CP on the other hand cannot operate in this mode so it was excluded from that build.

With your --static-cp all you are doing is not building osdp_pd.c into the share/static libraries. Are you trying to reduce the size of the library? if so this change makes sense, but it is too much of a micro optimisation :). Why not just build both everywhere? If you are really concerned with the size of the library, you can link you application statically which would provide best of both worlds IMO.

schmida2 commented 3 years ago

The code size is not my concern, so I might choose the wrong path here, My real concern is that I do not want to compile and target-install all these samples and tools in my productive build:

if (NOT CONFIG_OSDP_STATIC_PD AND NOT CONFIG_OSDP_STATIC_CP)
    add_subdirectory(osdpctl)
    add_subdirectory(test)
    add_subdirectory(samples/c)
    add_subdirectory(samples/cpp)
    add_subdirectory(doc)
    add_subdirectory(python)
endif()

I seem not to be the only one, see https://github.com/bojankoce/libosdp/commit/ec44187bf72ebbd667e9b872e0d4863f81ffbc44 ;-)

Could you imagine a different CONFIG flag for that purpose?

sidcha commented 3 years ago

Ahh, I see what you mean. Honestly, I didn't realise that the other subdirectories where causing so much pain :D

For this flag can we call it CONFIG_OSDP_LIB_ONLY or something in that line?

Also you need to adjust configure.sh accordingly and add --lib-only in such a way that it doesn't build anything but libosdp.a which is controlled by TARGETS variable. So something like:

diff --git a/configure b/configure
index f12888c..d7df952 100755
--- a/configure
+++ b/configure
@@ -133,6 +133,10 @@ if [[ ! -z "${OSDPCTL}" ]]; then
        TARGETS+=" osdpctl.elf"
 fi

+if [[ ! -z "${LIB_ONLY}" ]]; then
+       TARGETS=""
+fi
+
 ## Generate osdp_config.h
 echo "Generating osdp_config.h"
 cp src/osdp_config.h.in osdp_config.h
schmida2 commented 3 years ago

I made my changes, thanks for your help!