ra-mtp-ntnu / moto

A Python library for controlling Yaskawa MOTOMAN robots.
https://github.com/tingelst/moto
Apache License 2.0
38 stars 10 forks source link

`StateConnection.start()` does not completely protect againt unexpected states. #12

Open SebastianGrans opened 2 years ago

SebastianGrans commented 2 years ago

After establishing a StateConnection with the robot, there's a moment before any state packets have arrived. This causes the internal variables to be None which is annoying to deal with. PR #8 tried to fix this (and PR #4).

My initial pull request (#8) seemed to be working, but I discovered that it was just a lucky coincidence. I then "fixed it" and pushed directly, although there is still a potential issue.

My design idea was that StateConnection.start() would block until all the internal variables had be set by incoming messages. In the current state, it is done as follows:

if not self._initial_response.is_set() and (
                isinstance(self.robot_status(), RobotStatus)
                and any(isinstance(elem, JointFeedback) for elem in self._joint_feedback)
                and isinstance(self.joint_feedback_ex(), JointFeedbackEx)
            ):
                self._initial_response.set()

Problem

The problem is that the list self._joint_feedback is a None list of length 4. If the user instantiate Moto with two control groups, we could still run into the scenario that only StateConnection has received a message from only one of them. I.e. Moto.state.joint_feedback(2) would return None.

Suggested solution

When the user constructs an instance of Moto, they provide a list of control group definitions, and hence we know how many instances of JointFeedback the user is expecting to query.

Changes to StateConnection:

def __init__(self, ip_address: str, number_of_control_groups: int = MOT_MAX_GR):
    ...
    self._number_of_control_groups = number_of_control_groups
    self._check_function = any if self._number_of_control_groups == 0 else all
    ....

def _worker(self): 
    ...
    # Inside if initial response set statement 
    and self_check_function(isinstance(elem, JointFeedback) for elem in self._joint_feedback[:MOT_MAX_GR])

Add to Moto:

self._state_connection: StateConnection = StateConnection(self._robot_ip, len(control_group_defs)

For a user who instantiates Moto, the functions Moto.state.robot_status() and others will behave as expected.

And the user who instantiates StateConnection is at least partially protected from receiving None values.