Error message #10
Error message #10
Conversation
source/rpc/client.py
Outdated
| @@ -14,7 +14,7 @@ class O2x5xxRPCDevice(object): | |||
| Main API class | |||
| """ | |||
|
|
|||
| def __init__(self, address="192.168.0.69", api_path="/api/rpc/v1/"): | |||
| def __init__(self, address="192.168.1.69", api_path="/api/rpc/v1/"): | |||
There was a problem hiding this comment.
Leave this 192.168.0.69, which is the default static IP out of the box
There was a problem hiding this comment.
@Kottanidisan see comment
source/pcic/client.py
Outdated
| return data | ||
|
|
||
| else: | ||
| raise RuntimeError("Session is already open!") |
There was a problem hiding this comment.
There are 2 additional OperatingModes. Maybe raise RuntimeErrors with different messages depending on the Operating Mode. like:
if sessionstatus == 1:
raise RuntimeError("Sensor is in Parametrization Mode. Please close the ifmVisionAssistant and retry.")
elif .....
-1 Booting Mode @cfreundl is this right?
1 Parametrization Mode
There was a problem hiding this comment.
@Kottanidisan see comment
There was a problem hiding this comment.
Regarding the possible OperatingMode values: it is "0" (run mode), "1" (edit mode) or "2" (simulation mode). Simulation mode is only used internal for testing where you can feed images from the outside into the camera for processing similar to the snapshot processing in edit mode. Otherwise the device behaves just like it does in run mode, i.e. this would be a valid operating mode for PCIC communication.
I see two problems here: first, the ability to communicate with the device over PCIC is independent of whether there is a session opened and also whether the device has been put in edit mode. That's why I have doubts whether this method is a good place to perform this check. I think issue #7 should clarify the uses cases where an error message like this is helpful or even required (this is more a task for @Galoshi), maybe we can find a better location for this check then.
And as we are talking about the location, the impact of this check on the actual reception of PCIC output is huge. With every recv() call you now have to perform a full XMLRPC call first, i.e. one complete network roundtrip including an HTTP request. The latency that is introduced here might introduce performance issues if there is a continuously running application with a low evaluation time running on the device.
There was a problem hiding this comment.
After rereading the feature request in issue #7 I see that it already states that the error message should be returned when "an attempt is made to open a device", so this already sounds like the check is probably more suited in the __init__ method.
source/pcic/client.py
Outdated
|
|
||
| class Client(object): | ||
| def __init__(self, address, port): | ||
| def __init__(self, address, port, timeout_insec = 3): |
There was a problem hiding this comment.
Maybe you can use the timeout function from source/rpc/utils with an try except TimeoutError for the init fct?
The prefered way would be to use a decorator for the contructor init
There was a problem hiding this comment.
@Kottanidisan see comment
| @@ -6,17 +6,19 @@ | |||
| import json | |||
| import re | |||
| import io | |||
|
|
|||
| import o2x5xx | |||
There was a problem hiding this comment.
import from source instead of an installed library.
A customer cloning the repo for the first time does not own the o2x5xx lib
There was a problem hiding this comment.
@Kottanidisan see comment
There was a problem hiding this comment.
Just to confirm, the import statements need to be left as they are.
For testing changes like these I suggest to create a virtual environment where you install the o2x5xx-python packages and try a simple import o2x5xx afterwards. With the current state of the branch to be merged you will get the error
from o2x5xx import json, re, io
ImportError: cannot import name 'json' from partially initialized module 'o2x5xx' (most likely due to a circular import) (/tmp/venv/lib/python3.10/site-packages/o2x5xx-0.2-py3.10.egg/o2x5xx/__init__.py)
There was a problem hiding this comment.
@Kottanidisan Maybe a check like this can be done automatically by github for a branch?
| raise RuntimeError("Sensor is in Parametrization Mode. Please close the ifmVisionAssistant and retry.") | ||
|
|
||
| else: | ||
| raise RuntimeError("Sensor is in simulation mode at the moment.") |
There was a problem hiding this comment.
Looks good to me. Maybe @Galoshi can confirm whether this is in fact the desired check and whether this is sufficient.
Output error-messege if camera is already in a session