From 0ebe778aab2114ed2ad50d2bc03a679b0b1f48b8 Mon Sep 17 00:00:00 2001
From: c vw <dl1ycf@darc.de>
Date: Wed, 26 Jan 2022 11:55:52 +0100
Subject: [PATCH] Small cosmetic changes as preparation for next merge step.

---
 actions.c | 16 ++++++++--------
 gpio.c    |  5 +++--
 i2c.c     | 45 +++++++++++++++++++++++++++------------------
 sliders.c |  6 ++++--
 4 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/actions.c b/actions.c
index f7af4ee..5a95acd 100644
--- a/actions.c
+++ b/actions.c
@@ -188,9 +188,9 @@ ACTION_TABLE ActionTable[] = {
 // CW speed   (only MIDI)
 // CW side tone frequency (only MIDI)
 //
-  {CW_KEYER_KEYDOWN,    "KeyDown\n(keyer)",     NULL,           MIDI_KEY | CONTROLLER_SWITCH},
-  {CW_KEYER_SPEED,      "Speed\n(keyer)",       NULL,           MIDI_KNOB},
-  {CW_KEYER_SIDETONE,   "ST freq\n(keyer)",     NULL,           MIDI_KNOB},
+  {CW_KEYER_KEYDOWN,    "CW Key\n(keyer)",     NULL,           MIDI_KEY | CONTROLLER_SWITCH},
+  {CW_KEYER_SPEED,      "CW Speed\n(keyer)",       NULL,           MIDI_KNOB},
+  {CW_KEYER_SIDETONE,   "CW pitch\n(keyer)",     NULL,           MIDI_KNOB},
   {ACTIONS,		"",			NULL,		TYPE_NONE}
 };
 
@@ -568,7 +568,7 @@ int process_action(void *data) {
       }
       break;
     case CW_FREQUENCY:
-      value=KnobOrWheel(a, (double)cw_keyer_sidetone_frequency, 400.0, 1000.0, 10.0);
+      value=KnobOrWheel(a, (double)cw_keyer_sidetone_frequency, 300.0, 1000.0, 10.0);
       cw_keyer_sidetone_frequency=(int)value;
       g_idle_add(ext_vfo_update,NULL);
       break;
@@ -1227,8 +1227,8 @@ int process_action(void *data) {
         // however the range 0-127 is internally converted to 0-100 upstream
         //
         cw_keyer_speed=(127*a->val + 50)/100;
-	if (cw_keyer_speed <  1) cw_keyer_speed=1;
-	if (cw_keyer_speed > 99) cw_keyer_speed=99;
+        if (cw_keyer_speed <  1) cw_keyer_speed=1;
+        if (cw_keyer_speed > 99) cw_keyer_speed=99;
         g_idle_add(ext_vfo_update,NULL);
       }
       break;
@@ -1237,9 +1237,9 @@ int process_action(void *data) {
       if (a->mode==ABSOLUTE) {
         //
         // The MIDI keyer encodes the frequency as a value between 0 and 127,
-	// freq = 250 + 8*val
+        // freq = 250 + 8*val
         // however the range 0-127 is internally converted to 0-100 upstream
-	//
+        //
         cw_keyer_sidetone_frequency=250 + (254*a->val + 12)/25;
         g_idle_add(ext_vfo_update,NULL);
       }
diff --git a/gpio.c b/gpio.c
index c4afa7e..2c134e9 100644
--- a/gpio.c
+++ b/gpio.c
@@ -349,7 +349,7 @@ static void initialiseEpoch() {
   epochMilli = (uint64_t)ts.tv_sec * (uint64_t)1000    + (uint64_t)(ts.tv_nsec / 1000000L) ;
 }
 
-static uint32_t millis () {
+static unsigned int millis () {
   uint64_t now ;
   struct  timespec ts ;
   clock_gettime (CLOCK_MONOTONIC_RAW, &ts) ;
@@ -510,7 +510,7 @@ static void process_encoder(int e,int l,int addr,int val) {
   g_mutex_unlock(&encoder_mutex);
 }
 
-static void process_edge(int offset, enum ACTION_MODE value) {
+static void process_edge(int offset,int value) {
   gint i;
   unsigned int t;
   gboolean found;
@@ -552,6 +552,7 @@ static void process_edge(int offset, enum ACTION_MODE value) {
       found=TRUE;
       break;
     } else if(encoders[i].switch_enabled && encoders[i].switch_address==offset) {
+      //g_print("%s: found %d encoder %d switch\n",__FUNCTION__,offset,i);
       t=millis();
       //g_print("%s: found %d encoder %d switch value=%d t=%u\n",__FUNCTION__,offset,i,value,t);
       if (t<encoders[i].switch_debounce) {
diff --git a/i2c.c b/i2c.c
index fa1048c..ac2a2bc 100644
--- a/i2c.c
+++ b/i2c.c
@@ -26,6 +26,13 @@ unsigned int i2c_address_1=0X20;
 unsigned int i2c_address_2=0X23;
 
 static int fd;
+//
+// When reading the flags and ints registers of the
+// MCP23017, it is important that no other thread
+// (e.g., another instance of the interrupt service
+// routine), does this concurrently.
+// i2c_mutex guarantees this.
+//
 static GMutex i2c_mutex;
 
 #define SW_2  0X8000
@@ -83,29 +90,29 @@ void i2c_interrupt() {
   unsigned int ints;
   int i;
 
-  //
-  // The mutex guarantees that no MCP23017 registers are read by
-  // another instance of this function between reading "flags"
-  // and "ints".
-  // Perhaps we should determine the lock status and simply return if it is locked.
-  //
   g_mutex_lock(&i2c_mutex);
   for (;;) {
-    flags=read_word_data(0x0E);      // indicates which switch caused the interrupt
-                                     // More than one bit may be set if two input lines
-				     // changed state at the very same moment
-    if (flags == 0) break;           // "forever" loop is left if no interrups pending
-    ints=read_word_data(0x10);       // input lines at time of interrupt
-                                     // only those bits set in "flags" are meaningful!
+    flags=read_word_data(0x0E);
+    // bits in "flags" indicate which input lines triggered an interrupt
+    // Two interrupts occuring at about the same time can lead to multiple bits
+    // set in "flags" (or no bit set if interrupt has already been processed
+    // by another interrupt service routine). If we enter here (protected by
+    // the mutex), we handle all interrupts until no one is left (flags==0)
+    if (flags == 0) break;
+    ints=read_word_data(0x10);
 //g_print("%s: flags=%04X ints=%04X\n",__FUNCTION__,flags,ints);
-    for(i=0; i<16 && flags; i++) {   // leave loop if no bits left in flags.
-        if(i2c_sw[i] & flags) {
-          // The input line associated with switch #i has triggered an interrupt
+    // only those bits in "ints" matter where the corresponding position
+    // in "flags" is set. We have a PRESSED or RELEASED event depending on
+    // whether the bit in "ints" is set or clear.
+    for (i=0; i<16 && flags; i++) {  // leave loop if no bits left in "flags"
+      if(i2c_sw[i] & flags) {
 //g_print("%s: switches=%p sw=%d action=%d\n",__FUNCTION__,switches,i,switches[i].switch_function);
-          flags &= ~i2c_sw[i];       // clear *this* bit in flags
-          schedule_action(switches[i].switch_function, (ints & i2c_sw[i]) ? PRESSED : RELEASED, 0);
-	}
+        // The input line associated with switch #i has triggered an interrupt
+        // clear *this* bit in flags
+        flags &= ~i2c_sw[i];
+        schedule_action(switches[i].switch_function, (ints & i2c_sw[i]) ? PRESSED : RELEASED, 0);
       }
+    }
   }
   g_mutex_unlock(&i2c_mutex);
 }
@@ -165,6 +172,7 @@ void i2c_init() {
   if(write_byte_data(0x05,0xFF)<0) return;
 
   // flush any interrupts
+  g_mutex_lock(&i2c_mutex);
   int count=0;
   do {
     flags=read_word_data(0x0E);
@@ -176,6 +184,7 @@ void i2c_init() {
       }
     }
   } while(flags!=0);
+  g_mutex_unlock(&i2c_mutex);
 
 }
 #endif
diff --git a/sliders.c b/sliders.c
index 1631650..389007a 100644
--- a/sliders.c
+++ b/sliders.c
@@ -694,8 +694,10 @@ static void compressor_enable_cb(GtkWidget *widget, gpointer data) {
 void set_squelch(RECEIVER *rx) {
   g_print("%s\n",__FUNCTION__);
   //
-  // automatically enable/disable squelch
-  // if squelch value changed
+  // automatically enable/disable squelch if squelch value changed
+  // you can still enable/disable squelch via the check-box, but
+  // as soon the slider is moved squelch is enabled/disabled
+  // depending on the "new" squelch value
   //
   rx->squelch_enable = (rx->squelch > 0.5);
   setSquelch(rx);
-- 
2.45.2