Merge pull request #14 from jupyter-robotics/dev

Improve error handling and support multiple responses

Author: IsabelParedes

ipynao/_frontend.py

@@ -9,4 +9,4 @@
 """
 
 module_name = "ipynao"
-module_version = "^0.3.0"
+module_version = "^0.4.0"

ipynao/nao_robot.py

@@ -32,6 +32,8 @@ def _create_msg(self, method_name, *args, **kwargs):
         # convert tuple to list to avoid empty arg values
         data['args']    = list(args)
         data['kwargs']  = kwargs
+        data['requestID'] = self.widget.request_id
+        self.widget.request_id += 1
         return data
 
     def call_service(self, method_name, *args, **kwargs):
@@ -41,14 +43,19 @@ def call_service(self, method_name, *args, **kwargs):
     async def async_call_service(self, method_name, *args, **kwargs):
         data = self._create_msg(method_name, *args, **kwargs)
         self.widget.send(data)
+        request_id = data['requestID']
 
         try:
             self.output.clear_output()
             self.output.append_stdout('Calling service... \n')
-            await self.widget.wait_for_change('counter', self.output)
+            await self.widget.wait_for_change('counter', self.output, request_id)
         except Exception as e:
             return e
-        return self.widget.response['data']
+        
+        response = self.widget.response[request_id]['data']
+        del self.widget.response[request_id]
+        
+        return response
 
 
     def __getattr__(self, method_name):
@@ -69,7 +76,8 @@ class NaoRobotWidget(DOMWidget):
     connected = Unicode('Disconnected').tag(sync=True)
     status = Unicode('Not busy').tag(sync=True)
     counter = Integer(0).tag(sync=True)
-    response = None
+    response = {}
+    request_id = 0
 
 
     def __init__(self, **kwargs):
@@ -79,24 +87,35 @@ def __init__(self, **kwargs):
 
     def _handle_frontend_msg(self, model, msg, buffer):
         print('Received frontend msg: ', msg)
-        self.response = msg
+        request_id = msg['requestID']
+        self.response[request_id] = {
+            'isError': msg['isError'],
+            'data':    msg['data']
+        }
 
 
-    def wait_for_change(widget, value_name, output=Output()):
+    def wait_for_change(widget, value_name, output=Output(), request_id=0):
         future = asyncio.Future()
-        widget.response = None
+        widget.response[request_id] = {
+            'isError': False,
+            'data': None
+        }
 
         def get_value_change(change):
-            widget.unobserve(get_value_change, names=value_name)
-            if (widget.response != None):
-                if (widget.response['isError']):
-                    future.set_exception(Exception(widget.response['data']))
-                    output.append_stderr(widget.response['data'])
+            response = widget.response[request_id]
+
+            if (response['data'] != None): 
+                widget.unobserve(get_value_change, names=value_name)
+
+                if (response['isError']):
+                    future.set_exception(Exception(response['data']))
+                    output.append_stderr(response['data'])
                 else:
-                    future.set_result(widget.response['data'])
-                    output.append_stdout(widget.response['data'])
+                    future.set_result(response['data'])
+                    output.append_stdout(response['data'])
+
             else:
-                future.set_result(change)    
+                future.set_result(change) 
 
         widget.observe(get_value_change, names=value_name)
         return future    
@@ -107,18 +126,24 @@ def connect(self, ip_address='nao.local', port='80'):
         data['command'] = str('connect')
         data['ipAddress'] = str(ip_address)
         data['port'] = str(port)
+        data['requestID'] = self.request_id
         self.send(data)
+        self.request_id += 1
 
 
     def disconnect(self):
         data = {}
         data['command'] = str('disconnect')
+        data['requestID'] = self.request_id
         self.send(data)
+        self.request_id += 1
 
 
     def service(self, service_name, output=Output()):
         data = {}
         data['command'] = str('createService')
         data['service'] = str(service_name)
+        data['requestID'] = self.request_id
         self.send(data)
+        self.request_id += 1
         return NaoRobotService(self, service_name, output)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ipynao",
-  "version": "0.3.0",
+  "version": "0.4.0",
   "description": "A widget library for controlling Nao",
   "keywords": [
     "jupyter",

pyproject.toml

@@ -34,7 +34,7 @@ classifiers = [
 dependencies = [
     "ipywidgets>=7.0.0",
 ]
-version = "0.3.0"
+version = "0.4.0"
 
 [project.optional-dependencies]
 docs = [
@@ -104,7 +104,7 @@ file = [
 ]
 
 [tool.tbump.version]
-current = "0.3.0"
+current = "0.4.0"
 regex = "(?P<major>\\d+)\\.(?P<minor>\\d+)\\.(?P<patch>\\d+)((?P<channel>a|b|rc|.dev)(?P<release>\\d+))?"
 
 [tool.tbump.git]

requirements.txt

@@ -27,7 +27,7 @@ ipympl>=0.8.2
 ipycanvas>=0.9.1
 
 # Python: ipynao library for Nao robot
-ipynao>=0.3.0
+ipynao>=0.4.0
 
 # For examples with images
 Pillow

src/widget.ts

@@ -66,7 +66,7 @@ export class NaoRobotModel extends DOMWidgetModel {
     }
   }
 
-  async connect(ipAddress: string, port: string) {
+  async connect(ipAddress: string, port: string, requestID: number) {
     const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));
 
     this.changeStatus('Establishing connection');
@@ -97,25 +97,31 @@ export class NaoRobotModel extends DOMWidgetModel {
 
     // Handle connection failure
     if (!this.qiSession.isConnected()) {
+      this.disconnect();
       console.error('Connection to ', ipAddress, ' could not be established.');
-      this.changeStatus('Unavailable');
+      this.changeStatus(
+        'Connection to ' + ipAddress + ' could not be established.'
+      );
     }
   }
 
   disconnect() {
-    this.qiSession.disconnect();
+    if (this.qiSession && this.qiSession.isConnected()) {
+      this.qiSession.disconnect();
+    }
     this._services = {};
     this.set('connected', 'Disconnected');
     this.save_changes();
     this.changeStatus('Unavailable');
   }
 
-  private async checkConnection() {
+  private async checkConnection(requestID: number) {
     // Cannot reconnect without initial connection
     if (!this._ipAddress) {
       this.send({
         isError: true,
         data: 'Cannot connect without IP Address.',
+        requestID: requestID,
       });
       this.set('counter', this.get('counter') + 1);
       this.save_changes();
@@ -124,32 +130,47 @@ export class NaoRobotModel extends DOMWidgetModel {
 
     // Reconnect if possible
     if (!this.qiSession.isConnected()) {
-      await this.connect(this._ipAddress, this._port);
+      this.disconnect();
+      await this.connect(this._ipAddress, this._port, requestID);
     }
     return true;
   }
 
-  private async createService(serviceName: string) {
-    const isConnected: boolean = await this.checkConnection();
+  private async createService(serviceName: string, requestID: number) {
+    const isConnected: boolean = await this.checkConnection(requestID);
     if (!isConnected) {
       return;
     }
 
     // Skip if service exists already
-    if (this._services[serviceName] !== undefined) {
+    if (this._services[serviceName]) {
       console.log('Service ' + serviceName + ' exists.');
       return;
     }
 
     this.changeStatus('Creating service ' + serviceName);
     const servicePromise = this.qiSession.service(serviceName);
 
+    // TODO: This func is not async in the kernel. To show error messages
+    // the request ID is the next one which is used to call the service
     const naoService = await servicePromise
       .then((resolution: any) => {
+        this.send({
+          isError: false,
+          data: true, // TODO: resolution ?? true,
+          requestID: requestID + 1, // Note above
+        });
         return resolution;
       })
       .catch((rejection: string) => {
         this.changeStatus(rejection);
+        this.send({
+          isError: true,
+          data: rejection,
+          requestID: requestID + 1, // Note above
+        });
+        this.set('counter', this.get('counter') + 1);
+        this.save_changes();
         return rejection;
       });
 
@@ -164,27 +185,49 @@ export class NaoRobotModel extends DOMWidgetModel {
     serviceName: string,
     methodName: string,
     args: any,
-    _kwargs: any
+    _kwargs: any,
+    requestID: number
   ) {
-    const isConnected: boolean = await this.checkConnection();
+    const isConnected: boolean = await this.checkConnection(requestID);
     if (!isConnected) {
       return;
     }
 
     // Wait for service to become available
     const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));
+    this.changeStatus('Waiting for service ' + serviceName);
 
     // Timeout after ~10 seconds
     for (let i = 0; i < 100; i++) {
-      if (this._services[serviceName] !== undefined) {
+      if (this._services[serviceName]) {
         console.log('Service available after ', i / 10.0, ' seconds.');
+        this.changeStatus(serviceName + ' available');
         break;
       }
       await sleep(100);
     }
 
-    if (this._services[serviceName][methodName] === undefined) {
-      this.changeStatus(methodName + ' does not exist for ' + serviceName);
+    if (!this._services[serviceName]) {
+      this.changeStatus(serviceName + ' not available');
+      this.send({
+        isError: true,
+        data: serviceName + ' not available',
+        requestID: requestID,
+      });
+      this.set('counter', this.get('counter') + 1);
+      this.save_changes();
+      return;
+    }
+
+    if (!this._services[serviceName][methodName]) {
+      this.changeStatus(`${methodName} does not exist for ${serviceName}`);
+      this.send({
+        isError: true,
+        data: `${methodName} does not exist for ${serviceName}`,
+        requestID: requestID,
+      });
+      this.set('counter', this.get('counter') + 1);
+      this.save_changes();
       return;
     }
 
@@ -197,13 +240,15 @@ export class NaoRobotModel extends DOMWidgetModel {
         this.send({
           isError: false,
           data: resolution ?? true,
+          requestID: requestID,
         });
       })
       .catch((rejection: string) => {
         this.changeStatus(rejection);
         this.send({
           isError: true,
           data: rejection,
+          requestID: requestID,
         });
       });
 
@@ -216,23 +261,31 @@ export class NaoRobotModel extends DOMWidgetModel {
 
     switch (cmd) {
       case 'connect':
-        await this.connect(commandData['ipAddress'], commandData['port']);
+        await this.connect(
+          commandData['ipAddress'],
+          commandData['port'],
+          commandData['requestID']
+        );
         break;
 
       case 'disconnect':
         this.disconnect();
         break;
 
       case 'createService':
-        this.createService(commandData['service']);
+        await this.createService(
+          commandData['service'],
+          commandData['requestID']
+        );
         break;
 
       case 'callService':
         await this.callService(
           commandData['service'],
           commandData['method'],
           commandData['args'],
-          commandData['kwargs']
+          commandData['kwargs'],
+          commandData['requestID']
         );
         break;
     }