Improve BLE. To POP value from write_values

Author: hasumikin

mrbgems/picoruby-ble/example/peripheral-central/peripheral/app.rb

@@ -58,7 +58,7 @@ def initialize
   def heartbeat_callback
     @counter += 1
     temperature = ((27 - (@adc.read * 3.3 / (1<<12) - 0.706) / 0.001721) * 100).to_i
-    set_read_value(@temperature_handle, Utils.int16_to_little_endian(temperature))
+    push_read_value(@temperature_handle, Utils.int16_to_little_endian(temperature))
     if @counter == 10
       if @notification_enabled
         debug_puts "request_can_send_now_event"
@@ -70,7 +70,7 @@ def heartbeat_callback
       @led_on = !@led_on
       @led.write(@led_on ? 1 : 0)
     end
-    if write_value = get_write_value(@configuration_handle)
+    if write_value = pop_write_value(@configuration_handle)
       @notification_enabled = ( write_value == "\x01\x00" )
     end
   end

mrbgems/picoruby-ble/sig/ble.rbs

@@ -86,8 +86,8 @@ class BLE
   def heartbeat_callback: () -> void
   def blink_led: () -> void
   def gap_local_bd_addr: () -> String
-  def get_write_value: (Integer handle) -> (String|nil)
-  def set_read_value: (Integer handle, String value) -> void
+  def pop_write_value: (Integer handle) -> (String|nil)
+  def push_read_value: (Integer handle, String value) -> void
   def gap_connect: (String addr, Integer addr_time) -> Integer
 
   class Utils

mrbgems/picoruby-ble/src/ble.c

@@ -5,6 +5,18 @@
 
 #include "../../picoruby-machine/include/machine.h"
 
+/*
+ * Workaround: To avoid deadlock
+ * TODO: Maybe we need a critical section instead of these simple mutex
+ */
+static bool packet_mutex = false;
+static bool write_values_mutex = false;
+static bool heatbeat_flag = false;
+static bool packet_flag = false;
+
+static uint8_t *packet = NULL;
+static uint16_t packet_size = 0;
+
 #if defined(PICORB_VM_MRUBY)
 
 #include "mruby/ble.c"

mrbgems/picoruby-ble/src/mruby/ble.c

@@ -7,34 +7,25 @@ static mrb_state *_mrb = NULL;
 static mrb_value write_values;
 static mrb_value read_values;
 
-/*
- * Workaround: To avoid deadlock
- */
-static bool mutex_locked = false;
-static bool heatbeat_flag = false;
-static bool packet_flag = false;
-static uint8_t *packet = NULL;
-static uint16_t packet_size = 0;
-
 void
 BLE_push_event(uint8_t *data, uint16_t size)
 {
-  if (mutex_locked) return;
-  mutex_locked = true;
+  if (packet_mutex) return;
+  packet_mutex = true;
   packet_flag = true;
   packet_size = size;
   if (packet != NULL) {
     mrb_free(_mrb, packet);
   }
   packet = mrb_malloc(_mrb, packet_size);
   memcpy(packet, data, packet_size);
-  mutex_locked = false;
+  packet_mutex = false;
 }
 
 void
 BLE_heartbeat(void)
 {
-  if (mutex_locked) return;
+  if (packet_mutex) return;
   heatbeat_flag = true;
 }
 
@@ -45,7 +36,9 @@ BLE_write_data(uint16_t att_handle, const uint8_t *data, uint16_t size)
     return -1;
   }
   mrb_value write_value = mrb_str_new(_mrb, (const char *)data, size);
+  write_values_mutex = true;
   mrb_hash_set(_mrb, write_values, mrb_fixnum_value(att_handle), write_value);
+  write_values_mutex = false;
   return 0;
 }
 
@@ -75,26 +68,27 @@ static mrb_value
 mrb_pop_packet(mrb_state *mrb, mrb_value self)
 {
   mrb_value packet_value = mrb_nil_value();
-  if (mutex_locked || !packet_flag) return packet_value;
-  mutex_locked = true;
+  if (packet_mutex || !packet_flag) return packet_value;
+  packet_mutex = true;
   packet_flag = false;
   packet_value = mrb_str_new(mrb, (const char *)packet, packet_size);
   mrb_free(mrb, packet);
   packet = NULL;
-  mutex_locked = false;
+  packet_mutex = false;
   return packet_value;
 }
 
 static mrb_value
-mrb_get_write_value(mrb_state *mrb, mrb_value self)
+mrb_pop_write_value(mrb_state *mrb, mrb_value self)
 {
+  if (write_values_mutex) return mrb_nil_value();
   mrb_int handle;
   mrb_get_args(mrb, "i", &handle);
-  return mrb_hash_get(mrb, write_values, mrb_fixnum_value(handle));
+  return mrb_hash_delete_key(mrb, write_values, mrb_fixnum_value(handle));
 }
 
 static mrb_value
-mrb_set_read_value(mrb_state *mrb, mrb_value self)
+mrb_push_read_value(mrb_state *mrb, mrb_value self)
 {
   mrb_int handle;
   mrb_value read_value;
@@ -174,8 +168,8 @@ mrb_picoruby_ble_gem_init(mrb_state* mrb)
   mrb_define_method_id(mrb, class_BLE, MRB_SYM(_init), mrb__init, MRB_ARGS_REQ(1));
   mrb_define_method_id(mrb, class_BLE, MRB_SYM(hci_power_control), mrb_hci_power_control, MRB_ARGS_REQ(1));
   mrb_define_method_id(mrb, class_BLE, MRB_SYM(gap_local_bd_addr), mrb_gap_local_bd_addr, MRB_ARGS_NONE());
-  mrb_define_method_id(mrb, class_BLE, MRB_SYM(get_write_value), mrb_get_write_value, MRB_ARGS_REQ(1));
-  mrb_define_method_id(mrb, class_BLE, MRB_SYM(set_read_value), mrb_set_read_value, MRB_ARGS_REQ(2));
+  mrb_define_method_id(mrb, class_BLE, MRB_SYM(pop_write_value), mrb_pop_write_value, MRB_ARGS_REQ(1));
+  mrb_define_method_id(mrb, class_BLE, MRB_SYM(push_read_value), mrb_push_read_value, MRB_ARGS_REQ(2));
   mrb_define_method_id(mrb, class_BLE, MRB_SYM(pop_heartbeat), mrb_pop_heartbeat, MRB_ARGS_NONE());
   mrb_define_method_id(mrb, class_BLE, MRB_SYM(pop_packet), mrb_pop_packet, MRB_ARGS_NONE());
 

mrbgems/picoruby-ble/src/mrubyc/ble.c

@@ -4,34 +4,25 @@ static mrbc_value read_values = {.tt = MRBC_TT_NIL};
 #define NODE_BOX_SIZE 10
 #define VM_REGS_SIZE 110 // can be reduced?
 
-/*
- * Workaround: To avoid deadlock
- */
-static bool mutex_locked = false;
-static bool heatbeat_flag = false;
-static bool packet_flag = false;
-static uint8_t *packet = NULL;
-static uint16_t packet_size = 0;
-
 void
 BLE_push_event(uint8_t *data, uint16_t size)
 {
-  if (mutex_locked) return;
-  mutex_locked = true;
+  if (packet_mutex) return;
+  packet_mutex = true;
   packet_flag = true;
   packet_size = size;
   if (packet != NULL) {
     mrbc_raw_free(packet);
   }
   packet = mrbc_raw_alloc(packet_size);
   memcpy(packet, data, packet_size);
-  mutex_locked = false;
+  packet_mutex = false;
 }
 
 void
 BLE_heartbeat(void)
 {
-  if (mutex_locked) return;
+  if (packet_mutex) return;
   heatbeat_flag = true;
 }
 
@@ -41,7 +32,9 @@ BLE_write_data(uint16_t att_handle, const uint8_t *data, uint16_t size)
   if (att_handle == 0 || size == 0 || write_values.tt == MRBC_TT_NIL) {
     return -1;
   }
+  write_values_mutex = true;
   mrbc_value write_value = mrbc_string_new(NULL, (const void *)data, size);
+  write_values_mutex = false;
   return mrbc_hash_set(&write_values, &mrbc_integer_value(att_handle), &write_value);
 }
 
@@ -73,29 +66,33 @@ c_pop_heartbeat(mrbc_vm *vm, mrbc_value *v, int argc)
 static void
 c_pop_packet(mrbc_vm *vm, mrbc_value *v, int argc)
 {
-  if (mutex_locked || !packet_flag) {
+  if (packet_mutex || !packet_flag) {
     SET_NIL_RETURN();
     return;
   }
-  mutex_locked = true;
+  packet_mutex = true;
   packet_flag = false;
   mrb_value packet_value = mrbc_string_new(vm, (const char *)packet, packet_size);
   mrbc_raw_free(packet);
   packet = NULL;
-  mutex_locked = false;
+  packet_mutex = false;
   SET_RETURN(packet_value);
 }
 
 static void
-c_get_write_value(mrbc_vm *vm, mrbc_value *v, int argc)
+c_pop_write_value(mrbc_vm *vm, mrbc_value *v, int argc)
 {
+  if (write_values_mutex) {
+    SET_NIL_RETURN();
+    return;
+  }
   mrb_value handle = GET_ARG(1);
-  mrbc_value write_value = mrbc_hash_get(&write_values, &handle);
+  mrbc_value write_value = mrbc_hash_remove(&write_values, &handle);
   SET_RETURN(write_value);
 }
 
 static void
-c_set_read_value(mrbc_vm *vm, mrbc_value *v, int argc)
+c_push_read_value(mrbc_vm *vm, mrbc_value *v, int argc)
 {
   if (argc != 2) {
     mrbc_raise(vm, MRBC_CLASS(ArgumentError), "wrong number of arguments");
@@ -169,8 +166,8 @@ mrbc_ble_init(mrbc_vm *vm)
   mrbc_define_method(vm, class_BLE, "_init", c__init);
   mrbc_define_method(vm, class_BLE, "hci_power_control", c_hci_power_control);
   mrbc_define_method(vm, class_BLE, "gap_local_bd_addr", c_gap_local_bd_addr);
-  mrbc_define_method(vm, class_BLE, "get_write_value", c_get_write_value);
-  mrbc_define_method(vm, class_BLE, "set_read_value", c_set_read_value);
+  mrbc_define_method(vm, class_BLE, "pop_write_value", c_pop_write_value);
+  mrbc_define_method(vm, class_BLE, "push_read_value", c_push_read_value);
   mrbc_define_method(vm, class_BLE, "pop_heartbeat", c_pop_heartbeat);
   mrbc_define_method(vm, class_BLE, "pop_packet", c_pop_packet);
 

mrbgems/picoruby-shell/shell_executables/nmble.rb

@@ -22,11 +22,11 @@ class WifiConfigPeripheral < BLE
   CHARACTERISTIC_COUNTRY = 0x2AAD
   CHARACTERISTIC_WATCHDOG = 0x2AAE
 
-  def initialize
+  def initialize(name = "R2P2 BLE")
     # Setup BLE advertising data
     @adv_data = BLE::AdvertisingData.build do |a|
       a.add(BLUETOOTH_DATA_TYPE_FLAGS, APP_AD_FLAGS)
-      a.add(BLUETOOTH_DATA_TYPE_COMPLETE_LOCAL_NAME, "R2P2 BLE")
+      a.add(BLUETOOTH_DATA_TYPE_COMPLETE_LOCAL_NAME, name)
       a.add(BLUETOOTH_DATA_TYPE_COMPLETE_LIST_OF_16_BIT_SERVICE_CLASS_UUIDS, WIFI_CONFIG)
     end
 
@@ -63,26 +63,27 @@ def initialize
   def heartbeat_callback
     blink_led if @led_blink
 
-    if (ssid_value = get_write_value(@ssid_handle)) && ssid_value != @ssid
+    if (ssid_value = pop_write_value(@ssid_handle)) && ssid_value != @ssid
       @ssid = ssid_value
       debug_puts "Received SSID: #{ssid_value}"
     end
-    if (password_value = get_write_value(@password_handle)) && password_value != @password
+    if (password_value = pop_write_value(@password_handle)) && password_value != @password
       @password = password_value
       debug_puts "Received Password: ******"
     end
-    if (country_value = get_write_value(@country_handle)) && country_value != @country
+    if (country_value = pop_write_value(@country_handle)) && country_value != @country
       @country = country_value
       debug_puts "Received Country Code: #{country_value}"
     end
-    if (watchdog_value = get_write_value(@watchdog_handle)) && watchdog_value != @watchdog
+    if (watchdog_value = pop_write_value(@watchdog_handle)) && watchdog_value != @watchdog
       @watchdog = watchdog_value
       debug_puts "Received Watchdog: #{watchdog_value}"
     end
 
     if @ssid && @password #&& @country && @watchdog
       save_wifi_config
       @ssid = @password = @country = @watchdog = nil
+      puts "\nConfiguration saved. Please reboot the device.\n"
     end
   end
 
@@ -96,7 +97,10 @@ def save_wifi_config
         "watchdog" => (@watchdog == 'y')
       }
     }
-    File.open(ENV['WIFI_CONFIG_PATH'], "w") do |f| f.write YAML.dump(doc) end end
+    File.open(ENV['WIFI_CONFIG_PATH'], "w") do |f|
+      f.write YAML.dump(doc)
+    end
+  end
 
   def encrypt_password(password)
     cipher = MbedTLS::Cipher.new("AES-256-CBC")
@@ -138,8 +142,11 @@ def packet_callback(event_packet)
 puts "Open https://picoruby.github.io/wifi in a web browser\n"
 
 # Start BLE Peripheral
-peri = WifiConfigPeripheral.new
+require 'rng'
+name = "R2P2 BLE #{RNG.random_int.abs.to_s[0,4]}"
+peri = WifiConfigPeripheral.new(name)
 peri.debug = true
-puts "\nOpen https://picoruby.github.io/wifi\n"
+puts "\nOpen https://picoruby.github.io/wifi"
+puts "and find the device with name: '#{name}'\n"
 peri.start