EV3-G API: touch sensor add is_released by dwalton76 · Pull Request #367 · ev3dev/ev3dev-lang-python

dwalton76 · 2017-09-20T23:46:44Z

No description provided.

dwalton76 · 2017-09-21T00:00:43Z

So I added this for TouchSensor and was about to start on wait_for_pressed() and wait_for_released() (because those would be handy) but then noticed that a huge chunk of the sensors code is autogenerated from the templates...so the code in this PR will be over-written the next time some uses the templates to build core.py.

On our call the other night we kicked around the idea of life without the templates and autogen. I wasn't around when it was written but it sounds like things were changing often enough back then to make it worth it. Today though it doesn't feel worth it to me, the reasons being:

There have only been 5 commits to it all year, a good indication that things are not churning at nearly the rate that they used to
I find it difficult to use. I tried running it again today and it failed despite there not being any checkins since the last time I ran it and it worked.
It clutters the code base with # ~autogen generic-class classes.motor>currentClass and # ~autogen comments
It makes it way more complicated than it needs to be to add code like I have in this PR

The only thing we have to do to move away from it in ev3dev-lang-python is

remove the # ~autogen lines
remove the ev3dev-lang submodule

It could still be used for other ev3dev-lang-XYZ repos if they wish to use it.

Thoughts? Objections? +1s? :)

dwalton76 · 2017-09-21T00:07:09Z

Making sure @ddemidov @dlech @WasabiFan @ensonic see this

ddemidov · 2017-09-21T06:10:19Z

I am ok with this, just make sure to run the autogen one last time before you strip it off (I think your stretch-related commits in ev3dev-lang have not yet been applied).

WasabiFan · 2017-09-21T06:28:58Z

I think @dwalton76 was having issues running the script and I haven't sat down to debug it yet.

ddemidov · 2017-09-21T06:56:16Z

I just tried to do this myself and noticed that @dwalton76 already did this :). Sorry for the noise.

dwalton76 · 2017-09-21T12:19:52Z

cool, removal of ev3dev-lang and autogen via PR #368

…thon into ev3g-api-touch-sensor

dwalton76 · 2017-09-22T12:19:10Z

Am holding off on merging this in...will implement the following this weekend

wait_for_pressed()
wait_for_released()
wait_for_bumped()

WasabiFan · 2017-09-23T02:56:59Z

-
    #: Button state
    MODE_TOUCH = 'TOUCH'
+    MODES = ('TOUCH',)


Remove trailing comma

Also, can we use the other constant as the value here?

The trailing comma is needed for single entry tuples

>>> MODES = ('TOUCH',) >>> MODES[0] 'TOUCH' >>> >>> MODES = ('TOUCH') >>> MODES[0] 'T' >>>

Will change it to MODES = (MODE_TOUCH,)

😒 Python is... a special language.

WasabiFan · 2017-09-23T03:26:20Z

+        while True:
+            self._poll.poll(timeout_ms)
+
+            if wait_for_press:


Can we reduce this to a single Boolean expression?

Meaning if (wait_for_press and self.is_pressed) or (not wait_for_press and self.is_released)? I could go either way but I think the current layout is a tiny bit easier to read.

wait_for_press ^ self.is_released or self.is_pressed == wait_for_press? I think either should work, feel free to switch inversions for readability if that's useful.

ah I follow you now, fixed.

ddemidov · 2017-09-23T06:56:14Z

+        start_time = time.time()
+
+        if self.wait_for_pressed(timeout_ms):
+            timeout_ms -= int((time.time() - start_time) * 1000)


What happens here is timeout_ms is None? I think something like:

>>> None - 4 Traceback (most recent call last): File "<stdin>", line 1, in <module> TypeError: unsupported operand type(s) for -: 'NoneType' and 'int'

yep, good catch! Fixed now

dwalton76 · 2017-09-23T12:26:30Z

ok I think that is everything from the code review comments so am going to merge this one and we can do any further tweaking here via a new PR/issue.

This is for issue #361

EV3-G API: touch sensor add is_released

a7a2214

Merge branch 'develop' of https://github.com/dwalton76/ev3dev-lang-py…

91bc01e

…thon into ev3g-api-touch-sensor

ddemidov approved these changes Sep 22, 2017

View reviewed changes

dwalton76 added 2 commits September 23, 2017 01:30

Replace TABs with spaces

39fc7a9

TouchSensor: add wait_for_pressed, etc

bcc297e

WasabiFan reviewed Sep 23, 2017

View reviewed changes

MODES to use MODE_TOUCH constant

a93cb46

ddemidov reviewed Sep 23, 2017

View reviewed changes

dwalton76 added 2 commits September 23, 2017 11:25

Only decrement timeout_ms when it is not None

935c685

Simplify wait_for_press logic

69ea041

dwalton76 merged commit 4974253 into ev3dev:develop Sep 23, 2017

dwalton76 deleted the ev3g-api-touch-sensor branch September 23, 2017 12:26

Conversation

dwalton76 commented Sep 20, 2017

Uh oh!

dwalton76 commented Sep 21, 2017

Uh oh!

dwalton76 commented Sep 21, 2017

Uh oh!

ddemidov commented Sep 21, 2017

Uh oh!

WasabiFan commented Sep 21, 2017

Uh oh!

ddemidov commented Sep 21, 2017

Uh oh!

dwalton76 commented Sep 21, 2017

Uh oh!

dwalton76 commented Sep 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dwalton76 commented Sep 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants