Cliff Sensors Left unfinished

[scottcandy34]

I have found that the cliff sensors event has not been updated yet for the robot found here https://github.com/iRobotEducation/root-robot-ble-protocol?tab=readme-ov-file#device-20---cliff-sensor although it doesn't cover for the create3 I figured it out by reading the packets and implemented it below.

https://github.com/iRobotEducation/irobot-edu-python-sdk/blob/1a81d6f533937498bee43621b2c10fb2e6f81d1a/irobot_edu_sdk/robot.py#L267-L272

I have done an override class to implement it

class CliffSensor:
    def __init__(self):
        self.left = False
        self.left_front = False
        self.right = False
        self.right_front = False

class Create3(Create3): # Modified Create3 Package
    def __init__(self, backend):
        super().__init__(backend)

        self.cliff_sensor = CliffSensor()

    def when_cliff_sensor(self, condition: list[bool, bool, bool, bool], callback: Callable[[bool], Awaitable[None]]):
        """Register when cliff callback of type: async def fn(over_cliff: bool)."""
        self._when_cliff_sensor.append(Event(condition, callback))

    async def _when_cliff_sensor_handler(self, packet):
        self.cliff_sensor.disable_motors = packet.payload[4] != 0
        for event in self._when_cliff_sensor:
            self.cliff_sensor.right = packet.payload[4] & 0x01 != 0
            self.cliff_sensor.right_front = packet.payload[4] & 0x02 != 0
            self.cliff_sensor.left_front = packet.payload[4] & 0x04 != 0
            self.cliff_sensor.left = packet.payload[4] & 0x08 != 0

            # An empty condition list means to trigger the event on every occurrence.
            if not event.condition and (self.cliff_sensor.left or self.cliff_sensor.left_front or self.cliff_sensor.right_front or self.cliff_sensor.right):  # Any.
                await event.run(self)
                continue
            if len(event.condition) > 1 and ((event.condition[0] == self.cliff_sensor.left) and (event.condition[1] == self.cliff_sensor.left_front) and (event.condition[2] == self.cliff_sensor.right_front) and (event.condition[3] == self.cliff_sensor.right)):
                await event.run(self)

I only did a simple implementation of this. Can this be added to the create3.py, not sure what the sensors are for the root. I saw that it was left to do still. I did this back in January of 2023, so I thought I would share this. Here is the link to the issue that got fixed where I also shared some of this code. https://github.com/iRobotEducation/irobot-edu-python-sdk/issues/12

[shamlian]

If you'd like to fork the repo and then put in a PR targeting pre_0.5.2, I would likely approve it; alternatively, I can add "adding my own implementation" to my list of things to do. Let me know what you'd prefer.

[scottcandy34]

I can do that

[scottcandy34]

does the root have different sensor count or is it the same?

[shamlian]

Root has a single cliff sensor in the front center of the robot. I think if I were to write this, I would model the code after the touch sensors (since Create 3 has two while Root has four). If you don't feel comfortable, or if you just want to PR this to support the Create 3 robot only, I can work on abstracting it to both robots, later.

[scottcandy34]

That's what I thought from reading the Bluetooth documentation. I can model it after the touch sensors not an issue. But since there is not a center sensor for the create3 what should be the common ground name? just like the touch sensors share the front ones. Otherwise ill just make it front_left for common

[shamlian]

Ah, fair -- you're right, maybe I did a bad job with that abstraction. I think in this case, perfection is the enemy of good enough, and reusing that one arbitrarily is fine. I could also see the approach of adding a front member that is used only for Root, and then having a common get_is_cliffed() that returns true if any of the members are true, but a per-class get_cliff_sensors that returns a list of the proper values. This is a case of "do as I say, not as I do," clearly; I did not do that for the touch sensors. Probably, I should create an issue for the maintainer of the repo about that.

[scottcandy34]

Ok, apparently I don't get a choice for it. So it will be left that is common. This should be verified on a root. I doubled checked on the create3 and that works as intended. The pull request is here https://github.com/iRobotEducation/irobot-edu-python-sdk/pull/37

[scottcandy34]

Realized there should be some examples so here LOL https://github.com/iRobotEducation/irobot-edu-python-sdk/pull/39