Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error message #10

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

Error message #10

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
62d45d8
Added error message for when the device is already in a session
Kottanidisan Oct 11, 2023
dc26aaf
Changed wrong ip-address
Kottanidisan Oct 11, 2023
378ea9e
Remove unused code
Kottanidisan Oct 11, 2023
7844ece
Changed import
Kottanidisan Oct 18, 2023
8af7869
Added more RuntimeErrors for diffrent OperatingModes
Kottanidisan Oct 20, 2023
f19052c
Fixed datatyp error
Kottanidisan Oct 20, 2023
0b4759f
Query of the sessionstatus moved to __init__ method
Kottanidisan Oct 25, 2023
013306c
Merge branch 'ifm:master' into error-message
Kottanidisan Oct 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 21 additions & 12 deletions source/pcic/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,19 @@
import json
import re
import io

import o2x5xx
Copy link
Collaborator

@Galoshi Galoshi Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import from source instead of an installed library.
A customer cloning the repo for the first time does not own the o2x5xx lib

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kottanidisan see comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kottanidisan Maybe a check like this can be done automatically by github for a branch?


class Client(object):
def __init__(self, address, port):
def __init__(self, address, port, timeout_insec = 3):
Copy link
Collaborator

@Galoshi Galoshi Oct 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kottanidisan see comment

# open raw socket
self.pcicSocket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
#self.pcicSocket.settimeout(timeout_insec)
self.pcicSocket.connect((address, port))
self.recv_counter = 0
self.debug = False
self.debugFull = False

self.rpcdevice = o2x5xx.O2x5xxRPCDevice()

def __del__(self):
self.close()

Expand All @@ -27,15 +29,20 @@ def recv(self, number_bytes):
:param number_bytes: (int) length of bytes
:return: the data as bytearray
"""
data = bytearray()
while len(data) < number_bytes:
data_part = self.pcicSocket.recv(number_bytes - len(data))
if len(data_part) == 0:
raise RuntimeError("Connection to server closed")
data = data + data_part
self.recv_counter += number_bytes
return data

sessionstatus = self.rpcdevice.getParameter("OperatingMode")
if sessionstatus == "0":
data = bytearray()
while len(data) < number_bytes:
data_part = self.pcicSocket.recv(number_bytes - len(data))
if len(data_part) == 0:
raise RuntimeError("Connection to server closed")
data = data + data_part
self.recv_counter += number_bytes
return data

else:
raise RuntimeError("Session is already open!")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kottanidisan see comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


def close(self):
"""
Close the socket session with the device.
Expand All @@ -58,6 +65,8 @@ def read_next_answer(self):
answer_length = int(re.findall(r'\d+', str(answer))[1])
answer = self.recv(answer_length)
return ticket, answer[4:-2]



def read_answer(self, ticket):
"""
Expand Down
2 changes: 1 addition & 1 deletion source/rpc/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/"):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave this 192.168.0.69, which is the default static IP out of the box

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kottanidisan see comment

self.baseURL = "http://" + address + api_path
self.mainURL = self.baseURL + "com.ifm.efector/"
self.session = None
Expand Down