IOIO: make permission handling at least workable.

Author: chrisws

ioio/ioio/src/main/AndroidManifest.xml

@@ -3,9 +3,9 @@
   <uses-feature android:name="android.hardware.usb.accessory" android:required="true" />
   <uses-permission android:name="android.permission.USB_PERMISSION" />
   <application>
-    <receiver android:name="ioio.smallbasic.android.AccessoryPermissionCheck" android:exported="true">
+    <receiver android:name="ioio.smallbasic.android.AccessoryPermissionCheck" android:exported="false">
       <intent-filter>
-        <action android:name="android.hardware.usb.action.USB_PERMISSION" />
+        <action android:name="ioio.smallbasic.android.USB_PERMISSION" />
       </intent-filter>
     </receiver>
     <meta-data android:name="android.hardware.usb.action.USB_ACCESSORY_ATTACHED" android:resource="@xml/device_filter" />

ioio/ioio/src/main/java/ioio/smallbasic/IOLock.java

@@ -11,7 +11,7 @@
 
 public class IOLock<I> {
   private final Object mutex = new Object();
-  private static final int TIMEOUT_SECS = 5;
+  private static final int TIMEOUT_SECS = 10;
   private Consumer<I> consumer;
 
   public void invoke(Consumer<I> consumer) {
@@ -90,6 +90,7 @@ private CountDownLatch beginLatch() {
   private void endLatch(CountDownLatch latch) {
     try {
       if (!latch.await(TIMEOUT_SECS, TimeUnit.SECONDS)) {
+        IOService.setHardReset(true);
         throw new IOIOException("Timeout waiting for device");
       }
     } catch (InterruptedException e) {

ioio/ioio/src/main/java/ioio/smallbasic/IOService.java

@@ -3,6 +3,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import ioio.lib.api.IOIO;
 import ioio.lib.api.exception.ConnectionLostException;
@@ -13,6 +14,7 @@
 public class IOService implements IOIOLooperProvider  {
   private static final String TAG = "IOService";
   private static final int MAX_PINS = 46;
+  private static final AtomicBoolean HARD_RESET = new AtomicBoolean(false);
   private static IOService instance = null;
 
   private final ConnectionController connectionController;
@@ -27,13 +29,21 @@ private IOService() {
     usedPins = new Boolean[MAX_PINS + 1];
   }
 
+  public static boolean getHardReset() {
+    return HARD_RESET.get();
+  }
+
   public static IOService getInstance() {
     if (instance == null) {
       instance = new IOService();
     }
     return instance;
   }
 
+  public static void setHardReset(boolean hardReset) {
+    IOService.HARD_RESET.set(hardReset);
+  }
+
   public void addTask(IOTask ioTask) throws IOException {
     registerPin(ioTask.getPin());
     ioTasks.add(ioTask);

ioio/ioio/src/main/java/ioio/smallbasic/android/AccessoryPermissionCheck.java

@@ -12,27 +12,17 @@
 import android.os.Handler;
 import android.os.Looper;
 
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
-
 import ioio.lib.spi.Log;
 import ioio.smallbasic.IOIOException;
 
 public class AccessoryPermissionCheck extends BroadcastReceiver {
   private static final String TAG = AccessoryPermissionCheck.class.getSimpleName();
-  private static final String ACTION_USB_PERMISSION = "android.hardware.usb.action.USB_PERMISSION";
-  private static final String PERMISSION_ERROR = "Not permitted to use usb accessory.";
-  private static final long TIMEOUT_SECS = 30;
-  private final CountDownLatch latch;
-  private final AtomicBoolean permitted;
+  private static final String ACTION_USB_PERMISSION = "ioio.smallbasic.android.USB_PERMISSION";
+  private static final String PERMISSION_ERROR = "Not permitted";
 
   @TargetApi(Build.VERSION_CODES.TIRAMISU)
   public AccessoryPermissionCheck() {
     Log.d(TAG, "AccessoryPermissionCheck entered");
-    this.permitted = new AtomicBoolean(false);
-    this.latch = new CountDownLatch(1);
-
     UsbManager usbManager = (UsbManager) IOIOLoader.getContext().getSystemService(Context.USB_SERVICE);
     UsbAccessory[] accessories = usbManager.getAccessoryList();
     UsbAccessory accessory = (accessories == null ? null : accessories[0]);
@@ -43,43 +33,27 @@ public AccessoryPermissionCheck() {
       new Handler(Looper.getMainLooper()).post(() -> {
         Context context = IOIOLoader.getContext();
         IntentFilter filter = new IntentFilter(ACTION_USB_PERMISSION);
-        context.registerReceiver(this, filter, Context.RECEIVER_EXPORTED);
+        filter.setPriority(IntentFilter.SYSTEM_HIGH_PRIORITY - 1);
+        context.registerReceiver(this, filter, Context.RECEIVER_NOT_EXPORTED);
         int flags = PendingIntent.FLAG_IMMUTABLE;
         Intent intent = new Intent(ACTION_USB_PERMISSION);
         PendingIntent pendingIntent = PendingIntent.getBroadcast(context, 0, intent, flags);
         usbManager.requestPermission(accessory, pendingIntent);
       });
-      Log.d(TAG, "waiting for permission");
-      try {
-        if (!latch.await(TIMEOUT_SECS, TimeUnit.SECONDS)) {
-          permissionError();
-        } else {
-          IOIOLoader.getContext().unregisterReceiver(this);
-        }
-      }
-      catch (InterruptedException e) {
-        permissionError();
-        throw new IOIOException(PERMISSION_ERROR);
-      }
-      if (!permitted.get()) {
-        permissionError();
-      }
+      // for some reason using a latch caused an ANR here
+      Log.d(TAG, "requesting permission");
+      throw new IOIOException(PERMISSION_ERROR);
     }
   }
 
   @Override
-  public void onReceive(Context context, Intent intent) {
-    Log.d(TAG, intent.getAction());
-    synchronized (this) {
-      if (ACTION_USB_PERMISSION.equals(intent.getAction())) {
-        permitted.set(intent.getBooleanExtra(UsbManager.EXTRA_PERMISSION_GRANTED, false));
-        latch.countDown();
-      }
+  public synchronized void onReceive(final Context context, Intent intent) {
+    Log.d(TAG, "onReceive entered");
+    if (ACTION_USB_PERMISSION.equals(intent.getAction())) {
+      final BroadcastReceiver receiver = this;
+      new Handler(Looper.getMainLooper()).post(() -> {
+        context.unregisterReceiver(receiver);
+      });
     }
   }
-
-  private void permissionError() {
-    IOIOLoader.getContext().unregisterReceiver(this);
-    throw new IOIOException(PERMISSION_ERROR);
-  }
 }

ioio/ioio/src/main/java/ioio/smallbasic/android/AndroidLogger.java

@@ -1,14 +1,19 @@
 package ioio.smallbasic.android;
 
+import android.annotation.TargetApi;
+import android.os.Build;
 import android.util.Log;
 
 public class AndroidLogger implements ioio.lib.spi.Log.ILogger {
   public AndroidLogger() {
     super();
   }
 
+  @TargetApi(Build.VERSION_CODES.O)
   @Override
   public void write(int level, String tag, String message) {
-    Log.println(level, tag, message);
+    long id = Thread.currentThread().getId();
+    String text = tag + ": [#" + id + "] " + message;
+    Log.println(Log.ERROR, "smallbasic", text);
   }
 }

ioio/ioio/src/main/java/ioio/smallbasic/android/BluetoothConnection.java

@@ -41,14 +41,18 @@
 
 import ioio.lib.api.IOIOConnection;
 import ioio.lib.api.exception.ConnectionLostException;
-import ioio.lib.impl.FixedReadBufferedInputStream;
 import ioio.lib.spi.Log;
 import ioio.smallbasic.IOIOException;
+import ioio.smallbasic.IOService;
 
 class BluetoothConnection implements IOIOConnection {
   private static final String TAG = BluetoothConnection.class.getSimpleName();
+  private static final int HARD_RESET = 0x00;
+  private static final int SOFT_RESET = 0x01;
+  private static final int ESTABLISH_CONNECTION = 0x00;
+
   private ConnectionState state;
-  private InputStream inputStream;
+  private FixedReadBufferedInputStream inputStream;
   private OutputStream outputStream;
   private ParcelFileDescriptor fileDescriptor;
 
@@ -126,22 +130,27 @@ private boolean open() {
       inputStream = new FixedReadBufferedInputStream(new FileInputStream(fd), 1024);
       outputStream = new BufferedOutputStream(new FileOutputStream(fd), 1024);
 
-      // Soft-open the connection
-      outputStream.write(0x00);
+      // open the connection
+      if (IOService.getHardReset()) {
+        outputStream.write(HARD_RESET);
+        outputStream.write('I');
+        outputStream.write('O');
+        outputStream.write('I');
+        outputStream.write('O');
+        IOService.setHardReset(false);
+      } else {
+        outputStream.write(SOFT_RESET);
+      }
       outputStream.flush();
 
-      Log.i(TAG, "created streams");
-
-      // Ytai mentions http://code.google.com/p/android/issues/detail?id=20545
-      // which is now closed, but waiting still seems to be required.
-      //   "We're going to block now. We're counting on the IOIO to
-      //    write back a byte, or otherwise we're locked until
-      //    physical disconnection"
-      while (inputStream.read() != 1) {
-        wait(1000);
+      // ensure IOIOProtocol.run() first see's ESTABLISH_CONNECTION(0x00) and not SOFT_RESET(0x01)
+      // to avoid an NPE in IncomingState.handleSoftReset(). This method relies on initialisation in
+      // IncomingState.handleEstablishConnection
+      if (inputStream.positionTo(ESTABLISH_CONNECTION)) {
+        Log.i(TAG, "created streams");
+      } else {
+        Log.i(TAG, "created streams - failed to position data");
       }
-
-      Log.i(TAG, "success");
       result = true;
     } catch (Exception e) {
       Log.v(TAG, "Failed to open streams", e);

ioio/ioio/src/main/java/ioio/smallbasic/android/FixedReadBufferedInputStream.java

@@ -0,0 +1,99 @@
+package ioio.smallbasic.android;
+
+import java.io.BufferedInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * Similar to a {@link BufferedInputStream}, but guarantees that all reads from the source stream
+ * are exactly the specified size of the buffer.
+ * It turns out, {@link BufferedInputStream} does not actually have such a guarantee.
+ */
+public class FixedReadBufferedInputStream extends InputStream {
+  private int bufferIndex_ = 0;
+  private int validData_ = 0;
+  private final byte[] buffer_;
+  private final InputStream source_;
+
+  public FixedReadBufferedInputStream(InputStream source, int size) {
+    buffer_ = new byte[size];
+    source_ = source;
+  }
+
+  @Override
+  public int available() throws IOException {
+    return validData_ - bufferIndex_;
+  }
+
+  @Override
+  public void close() throws IOException {
+    source_.close();
+  }
+
+  @Override
+  public int read(byte[] buffer, int offset, int length) throws IOException {
+    fillIfEmpty();
+    if (validData_ == -1) {
+      return -1;
+    }
+    length = Math.min(length, validData_ - bufferIndex_);
+    System.arraycopy(buffer_, bufferIndex_, buffer, offset, length);
+    bufferIndex_ += length;
+    return length;
+  }
+
+  @Override
+  public int read(byte[] buffer) throws IOException {
+    return read(buffer, 0, buffer.length);
+  }
+
+  @Override
+  public int read() throws IOException {
+    fillIfEmpty();
+    if (validData_ == -1) {
+      return -1;
+    }
+    return buffer_[bufferIndex_++] & 0xFF;
+  }
+
+  @Override
+  public long skip(long byteCount) throws IOException {
+    long skipped = 0;
+    while (byteCount > 0) {
+      fillIfEmpty();
+      if (validData_ == -1) {
+        return skipped;
+      }
+      int count = (int) Math.min(available(), byteCount);
+      byteCount -= count;
+      bufferIndex_ += count;
+      skipped += count;
+    }
+    return skipped;
+  }
+
+  protected boolean positionTo(int value) throws IOException {
+    fillIfEmpty();
+    if (validData_ != -1) {
+      while (bufferIndex_ < buffer_.length) {
+        if (buffer_[bufferIndex_] == value) {
+          break;
+        } else {
+          bufferIndex_++;
+        }
+      }
+    }
+    return validData_ != -1;
+  }
+
+  private void fill() throws IOException {
+    bufferIndex_ = 0;
+    validData_ = source_.read(buffer_);
+  }
+
+  private void fillIfEmpty() throws IOException {
+    while (available() == 0 && validData_ != -1) {
+      fill();
+    }
+  }
+}