irmp: rework shared library (style, reliability, TODO items)

Address several style nits in the previous implementation of the PC
library, but keep the core as is to simplify future upstream tracking.

Eliminate camel case identifiers, and in(?)/out prefixes for variables,
only keep s_ for global(?) variables. Fixup whitespace, reduce a little
indentation where appropriate. Separate variable declaration from later
assignments and updates to improve readability. Use C style comments.

Drop initializer values for .bss variables. Decorate the declaration as
well as implementation of routines for symbol export. Improve robustness
of name lookups in the list of known protocols.

Prefer native C language data types in the public API. Normalize data in
the wrapper so that application code need not care. Make the byte buffer
API for IR frame detection optional. The API is limited and overloaded
at the same time, and may need more consideration.

Extend comments in spots which are essential for proper operation, or
which encode non-obvious details of the build system. Control visibility
of public API identifiers on non-Windows platforms, too. Rephrase the
doxygen comments for more formal API documentation. Discuss limitations
in the current implementation. Keep a TODO list in the source code.

2 files changed, 209 insertions, 121 deletions

diff --git a/irmp/irmp-main-sharedlib.c b/irmp/irmp-main-sharedlib.c
index 824a05a..baa65c9 100644
--- a/irmp/irmp-main-sharedlib.c
+++ b/irmp/irmp-main-sharedlib.c
@@ -1,5 +1,5 @@
-/*---------------------------------------------------------------------------------------------------------------------------------------------------
- * irmpharedLib.h
+/*
+ * irmp-main-sharedlib.c
  *
  * Copyright (c) 2009-2019 Frank Meyer - frank(at)fli4l.de
  * Copyright (c) 2009-2019 René Staffen - r.staffen(at)gmx.de
@@ -8,98 +8,162 @@
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
- *---------------------------------------------------------------------------------------------------------------------------------------------------
  */
 
+/*
+ * Declare the library's public API first. Prove it's consistent and
+ * complete as a standalone header file.
+ */
+#include "irmp-main-sharedlib.h"
 
+#include <stdlib.h>
+#include <string.h>
+
+/*
+ * Include the IRMP core logic. This approach is required because of
+ * static variables which hold internal state. The core logic started
+ * as an MCU project where resources are severely constrained.
+ */
 #include "irmp.h"
 #include "irmp.c"
 
+/*
+ * The remaining source code implements the PC library, which accepts
+ * sample data from API callers, and provides detector results as they
+ * become available after seeing input data.
+ *
+ * TODO items, known constraints
+ * - Counters in the IRMP core logic and the library wrapper are 32bit
+ *   only. In the strictest sense they only need to cover the span of
+ *   an IR frame. In the PC side library case they need to cover "a
+ *   detection phase", which happens to be under calling applications'
+ *   control. The library shall not mess with the core's internal state,
+ *   and may even not be able to reliably tell whether detection of a
+ *   frame started in the core. Fortunately the 32bit counters only roll
+ *   over after some 2.5 days at the highest available sample rate. So
+ *   this limitation is not a blocker.
+ * - The IRMP core keeps internal state in global variables. Which is
+ *   appropriate for MCU configurations. For the PC library use case
+ *   this constraint prevents concurrency, only a single data stream
+ *   can get processed at any time. This limitation can get addressed
+ *   later, making the flexible and featureful IRMP detection available
+ *   in the first place is considered highly desirable, and is a great
+ *   improvement in itself.
+ * - The detection of IR frames from buffered data is both limited and
+ *   complicated at the same time. The routine re-uses the caller's
+ *   buffer _and_ internal state across multiple calls. Thus windowed
+ *   operation over a larger set of input data is not available. The
+ *   API lacks a flag for failed detection, thus applications need to
+ *   guess from always returned payload data.
+ * - Is it worth adding a "detection in progress" query to the API? Is
+ *   the information available to the library wrapper, and reliable?
+ *   Shall applications be able to "poll" the started, and completed
+ *   state for streamed operation including periodic state resets which
+ *   won't interfere with pending detection? (It's assumed that this
+ *   is only required when feeding single values in individual calls is
+ *   found to be rather expensive.
+ * - Some of the result data reflects the core's internal presentation
+ *   while there is no declaration in the library's API. This violates
+ *   API layers, and needs to get addressed properly.
+ * - The IRMP core logic (strictly speaking the specific details of
+ *   preprocessor symbol arrangements in the current implementation)
+ *   appears to assume either to run on an MCU and capture IR signals
+ *   from hardware pins, falling back to AVR if no other platform got
+ *   detected. Or assumes to run on a (desktop) PC, and automatically
+ *   enables ANALYZE mode, which results in lots of stdio traffic that
+ *   is undesirable for application code which uses the shared library
+ *   for strict detection purposes but no further analysis or research.
+ *   It's a pity that turning off ANALYZE switches to MCU mode, and that
+ *   keeping ANALYZE enabled but silencing the output is rather messy
+ *   and touches the innards of the core logic (the irmp.c source file
+ *   and its dependency header files).
+ */
 
-#ifndef IRMP_DLLEXPORT
-
-#if defined WIN32 && defined _MSC_VER
-# define IRMP_DLLEXPORT __declspec(dllexport)
-#else
-# define IRMP_DLLEXPORT
+#ifndef ARRAY_SIZE
+#  define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0]))
 #endif
-#endif // !IRMP_DLLEXPORT
-
-#include "irmp-main-sharedlib.h"
-
 
+static uint32_t s_end_sample;
 
-static uint32_t s_endSample = 0;
-
-uint32_t IRMP_GetSampleRate(void) {
-    return F_INTERRUPTS;
+IRMP_DLLEXPORT uint32_t irmp_get_sample_rate(void)
+{
+	return F_INTERRUPTS;
 }
 
-
-void IRMP_Reset(void) {
-    IRMP_PIN = 0xff;
-    IRMP_DATA data;
-    int i;
-    for (i = 0; i < (int)(( F_INTERRUPTS )); i++)  // long pause of 1s
-    {
-        (void)irmp_ISR();
-    }
-    (void)irmp_get_data(&data);
-    time_counter = 0;
-    s_startBitSample = 0;
-    s_curSample = 0;
-    s_endSample = 0;
+IRMP_DLLEXPORT void irmp_reset_state(void)
+{
+	size_t i;
+	IRMP_DATA data;
+
+	/*
+	 * Provide the equivalent of 1s idle input signal level. Then
+	 * drain any potentially accumulated result data. This clears
+	 * the internal decoder state.
+	 */
+	IRMP_PIN = 0xff;
+	i = F_INTERRUPTS;
+	while (i-- > 0) {
+		(void)irmp_ISR();
+	}
+	(void)irmp_get_data(&data);
+
+	time_counter = 0;
+	s_startBitSample = 0;
+	s_curSample = 0;
+	s_end_sample = 0;
 }
 
+IRMP_DLLEXPORT int irmp_add_one_sample(int sample)
+{
+	int ret;
 
-uint32_t IRMP_AddSample(const uint8_t i_sample) {
-    IRMP_PIN = i_sample;
-    uint_fast8_t r = irmp_ISR();
-    if (r) {
-        s_endSample = s_curSample;
-        return 1;
-    }
-    s_curSample++;
-    return 0;
+	IRMP_PIN = sample ? 0xff : 0x00;
+	ret = irmp_ISR() ? 1 : 0;
+	s_end_sample = s_curSample++;
+	return ret;
 }
 
-
-uint32_t IRMP_GetData(IRMP_DataExt* o_data) {
-
-    IRMP_DATA d;
-    if (irmp_get_data(&d))
-    {  
-        o_data->address      = d.address;
-        o_data->command      = d.command;
-        o_data->protocol     = d.protocol;
-        o_data->protocolName = IRMP_GetProtocolName(d.protocol);
-        o_data->flags        = d.flags;
-        o_data->startSample  = s_startBitSample;
-        o_data->endSample    = s_endSample;
-        return TRUE;
-    }
-    return FALSE;
+IRMP_DLLEXPORT int irmp_get_result_data(struct irmp_result_data *data)
+{
+	IRMP_DATA d;
+
+	if (!irmp_get_data(&d))
+		return 0;
+
+	data->address = d.address;
+	data->command = d.command;
+	data->protocol = d.protocol;
+	data->protocol_name = irmp_get_protocol_name(d.protocol);
+	data->flags = d.flags;
+	data->start_sample = s_startBitSample;
+	data->end_sample = s_end_sample;
+	return 1;
 }
 
-
-IRMP_DataExt IRMP_Detect(const uint8_t* i_buff, uint32_t i_len) {
-    IRMP_DataExt ret = { 0 };
-    while (s_curSample < i_len) {
-        if (IRMP_AddSample(i_buff[s_curSample])) {
-            IRMP_GetData(&ret);
-            return ret;
-        }
-    }
-    return ret;
+#if WITH_IRMP_DETECT_BUFFER
+IRMP_DLLEXPORT struct irmp_result_data irmp_detect_buffer(const uint8_t *buff, size_t len)
+{
+	struct irmp_result_data ret;
+
+	memset(&ret, 0, sizeof(ret));
+	while (s_curSample < len) {
+		if (irmp_add_one_sample(buff[s_curSample])) {
+			irmp_get_result_data(&ret);
+			return ret;
+		}
+	}
+	return ret;
 }
+#endif
 
+IRMP_DLLEXPORT const char *irmp_get_protocol_name(uint32_t protocol)
+{
+	const char *name;
 
-const char* IRMP_GetProtocolName(uint32_t i_protocol) {
-    if (i_protocol < IRMP_N_PROTOCOLS) {
-        return irmp_protocol_names[i_protocol];
-    }
-    else {
-        return "unknown";
-    }
+	if (protocol >= ARRAY_SIZE(irmp_protocol_names))
+		return "unknown";
+	name = irmp_protocol_names[protocol];
+	if (!name || !*name)
+		return "unknown";
+	return name;
 }
-
diff --git a/irmp/irmp-main-sharedlib.h b/irmp/irmp-main-sharedlib.h
index 27f8add..05e7b53 100644
--- a/irmp/irmp-main-sharedlib.h
+++ b/irmp/irmp-main-sharedlib.h
@@ -1,5 +1,5 @@
-/*---------------------------------------------------------------------------------------------------------------------------------------------------
- * irmpharedLib.h
+/*
+ * irmp-main-sharedlib.h
  *
  * Copyright (c) 2009-2019 Frank Meyer - frank(at)fli4l.de
  * Copyright (c) 2009-2019 René Staffen - r.staffen(at)gmx.de
@@ -8,84 +8,108 @@
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
- *---------------------------------------------------------------------------------------------------------------------------------------------------
  */
 
-#ifndef IRMP_SHARED_H
-#define IRMP_SHARED_H
+#ifndef IRMP_SHAREDLIB_H
+#define IRMP_SHAREDLIB_H
 
 #include <stdint.h>
+#include <stdlib.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
+/* Export the public API routines. */
 #ifndef IRMP_DLLEXPORT
-
-#if defined WIN32 && defined _MSC_VER
-# define IRMP_DLLEXPORT __declspec(dllimport)
-#else
-# define IRMP_DLLEXPORT
+#  if defined WIN32 && defined _MSC_VER
+#    define IRMP_DLLEXPORT __declspec(dllexport)
+#  else
+#    define IRMP_DLLEXPORT __attribute__((visibility("default")))
+#  endif
 #endif
-#endif // !IRMP_DLLEXPORT
 
+/* Part of the library API is optional. */
+#define WITH_IRMP_DETECT_BUFFER 0
 
 /**
- * result data
+ * @brief IR decoder result data at the library's public API.
  */
-typedef struct
-{ 
-    uint32_t    protocol;     ///< protocol, e.g. NEC_PROTOCOL
-    const char* protocolName; ///< name of the protocol
-    uint32_t    address;      ///< address
-    uint32_t    command;      ///< command
-    uint32_t    flags;        ///< flags currently only repetition (bit 0)
-    uint32_t    startSample;  ///< the sampleindex there the detected command started
-    uint32_t    endSample;    ///< the sampleindex there the detected command ended
-}  IRMP_DataExt;
-
+struct irmp_result_data {
+	uint32_t protocol;	/**!< protocol, e.g. NEC_PROTOCOL */
+	const char *protocol_name;	/**!< name of the protocol */
+	uint32_t address;	/**!< address */
+	uint32_t command;	/**!< command */
+	uint32_t flags;		/**!< flags currently only repetition (bit 0) */
+	uint32_t start_sample;	/**!< the sampleindex there the detected command started */
+	uint32_t end_sample;	/**!< the sampleindex there the detected command ended */
+};
+
+#define IRMP_DATA_FLAG_REPETITION	(1 << 0)
 
 /**
- * Returns the sample rate for that the irmp library was compiled for.
- * Any data provided has resamble  this sample rate or detection will fail.
+ * @brief Query the IRMP library's configured sample rate.
+ *
+ * The internally used sample rate is a compile time option. Any data
+ * that is provided at runtime needs to match this rate, or detection
+ * will fail.
  */
-IRMP_DLLEXPORT  uint32_t IRMP_GetSampleRate(void);
+IRMP_DLLEXPORT uint32_t irmp_get_sample_rate(void);
 
 /**
- * Resets the internal state of the detector 
- * This has to be called before start processing data.
+ * @brief Reset internal decoder state.
+ *
+ * This must be called before data processing starts.
  */
-IRMP_DLLEXPORT  void IRMP_Reset(void);
+IRMP_DLLEXPORT void irmp_reset_state(void);
 
 /**
- * Feeds a single sample into the detecor.
- * Returns 1 if a ir command was detected.
- * Use IRMP_GetData to retrieve the data.
- * Make sure, that Reset was called before adding first Sample.
+ * @brief Feed an individual sample to the detector.
+ *
+ * See @ref irmp_get_result_data() for result retrieval when detection
+ * of an IR frame completes. Make sure @ref irmp_reset_state() was
+ * called before providing the first sample.
+ *
+ * @param[in] sample The pin value to feed to the detector.
+ *
+ * @returns Non-zero when an IR frame was detected.
  */
-IRMP_DLLEXPORT  uint32_t IRMP_AddSample(const uint8_t i_sample);
-
+IRMP_DLLEXPORT int irmp_add_one_sample(int sample);
 
+#if WITH_IRMP_DETECT_BUFFER
 /**
- * Proceses the given buffer and stops on the first found command and returns it data.
- * Further calls resume the processing at the previously stopped position.
- * Make sure, that Reset was called before first calling Detect.
+ * @brief Process the given buffer until an IR frame is found.
+ *
+ * Stops at the first detected IR frame, and returns its data. Subsequent
+ * calls resume processing at the previously stopped position. Make sure
+ * @ref irmp_reset_state() was called before the first detect call.
+ *
+ * @param[in] buf Pointer to the data buffer.
+ * @param[in] len Number of samples in the Buffer.
  */
-IRMP_DLLEXPORT  IRMP_DataExt IRMP_Detect(const uint8_t* i_buff, uint32_t i_len);
-
+IRMP_DLLEXPORT struct irmp_result_data irmp_detect_buffer(const uint8_t *buf, size_t len);
+#endif
 
 /**
- * If a valid IR frame was detected the provided output structure is filled
- * \returns 1 if data was available, 0 else
+ * @brief Query result data after detection succeeded.
+ *
+ * @param[out] data The caller provided result buffer.
+ *
+ * @returns Non-zero if data was available, zero otherwise.
  */
-IRMP_DLLEXPORT  uint32_t IRMP_GetData(IRMP_DataExt* o_data);
-
-/** returns the the name of the given protocol number */
-IRMP_DLLEXPORT  const char* IRMP_GetProtocolName(uint32_t i_protocol);
+IRMP_DLLEXPORT int irmp_get_result_data(struct irmp_result_data *data);
 
+/**
+ * @brief Resolve the protocol identifer to the protocol's name.
+ *
+ * @param[in] protocol The numerical identifier.
+ *
+ * @returns A pointer to the string literal, or #NULL in case of failure.
+ */
+IRMP_DLLEXPORT const char *irmp_get_protocol_name(uint32_t protocol);
 
 #ifdef __cplusplus
 }
 #endif
 
-#endif // IRMP_SHARED_H
+#endif