Added Cliff sensor support #37
Conversation
common is `.cliff_sensor.left`
| @@ -297,7 +313,7 @@ def when_touched(self, condition: list[bool, bool, bool, bool], callback: Callab | |||
| """Register when touch callback of type: async def fn(front_left: bool, front_right: bool, back_left: bool, back_right: bool).""" | |||
| self._when_touched.append(Event(condition, callback)) | |||
|
|
|||
| def when_cliff_sensor(self, condition: list[bool], callback: Callable[[bool], Awaitable[None]]): | |||
| def when_cliff_sensor(self, condition: list[bool, bool, bool, bool], callback: Callable[[bool], Awaitable[None]]): | |||
There was a problem hiding this comment.
This is consistent with the current library but makes me realize I may need to do something about type hinting for the case where different robots have different numbers of arguments (this comment is more for me than you 😜 )
| for event in self._when_cliff_sensor: | ||
| # TODO: Add trigger | ||
| await event.run(self) | ||
| for event in self._when_cliff_sensor: |
There was a problem hiding this comment.
Hey, I'm sorry, but this doesn't work like the rest of the system does, where event conditions are masks, and not exact matches. (Also, this doesn't work at all on Root.) I think we had this conversation some time ago, when you were asking about how the bumper events work. I am going to open a new issue (#40) about how/whether events should latch, or if we should create a way to define events with exact matches (#41), but for now, I want to keep it consistent.
I think it should all actually look like this:
for event in self._when_cliff_sensor:
# An empty condition list means to trigger the event on every occurrence.
if not event.condition and self.cliff_sensor.disable_motors: # Any.
await event.run(self)
elif len(event.condition) > 0 and len(event.condition) < 3:
if (event.condition[0] == self.cliff_sensor.disable_motors):
await event.run(self)
elif len(event.condition) > 3:
if ((event.condition[0] and self.cliff_sensor.left) or
(event.condition[1] and self.cliff_sensor.front_left) or
(event.condition[2] and self.cliff_sensor.front_right) or
(event.condition[3] and self.cliff_sensor.right)):
await event.run(self)
There was a problem hiding this comment.
I can PR into your branch, or else if you're willing, could you replace this block?
There was a problem hiding this comment.
That makes sense for the Root, I figured it might be wrong. I can change that, as for the and's to or's I have tried that but it would not work at all after that. So I left it as is (with the and's). Did you have a different experience?
There was a problem hiding this comment.
This block is working on both the Create 3 and Root RT1 that I tested it on, with the installable SDK. How are you testing, with the web interface?
There was a problem hiding this comment.
Interesting. Yes the web, but currently I'm not near the bot so I can't test again.
There was a problem hiding this comment.
I am happy to wait until you've next got a chance to test. Would be interested to know why it didn't work for you.
There was a problem hiding this comment.
That would be Monday. I'll check then, its in my office. Same and if its still the same now since you changed the any call. Which could of been the problem.
There was a problem hiding this comment.
No worries at all, I appreciate you taking the initiative to get the cliff event rounded out. Have a great weekend!
There was a problem hiding this comment.
IT WORKED! It worked fine. apparently I had some bad logic. I mean my code did work for the create3 but your changes are definingly for the better since it now works for Root and Create3. That makes me feel better knowing the readability of the logic makes sense now.
Support for create3 and root