Skip to content

Remove "connected" attribute#402

Merged
dwalton76 merged 9 commits intodevelopfrom
remove-connected-attribute
Nov 14, 2017
Merged

Remove "connected" attribute#402
dwalton76 merged 9 commits intodevelopfrom
remove-connected-attribute

Conversation

@WasabiFan
Copy link
Copy Markdown
Member

@WasabiFan WasabiFan commented Oct 8, 2017

Fixes #372

I haven't tested this on-device yet.

Comment thread ev3dev/__init__.py Outdated
yield f


class DeviceNotFoundException(Exception):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

would do class DeviceNotFound(Exception)

Comment thread ev3dev/__init__.py Outdated
self._path = None
self._device_index = None
self.connected = False
raise DeviceNotFoundException("No device matching the given requirements was found.")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"%s is not connected" % self would let the user know exactly which device isn't connected

Comment thread ev3dev/__init__.py Outdated
# rather than a library error. If that isn't the case, at a minimum the underlying
# error info will be printed for debugging.
raise Exception("%s is no longer connected" % self) from driver_error
raise DeviceNotFoundException("%s is no longer connected" % self) from driver_error
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

s/DeviceNotFoundException/DeviceNotFound/

Comment thread tests/api_tests.py Outdated

d = ev3.Device('this-does-not-exist')
self.assertFalse(d.connected)
with self.assertRaises(ev3.DeviceNotFoundException):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same for these two

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

other than that it looks good to me

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

😟 Why didn't our tests catch this? That's... quite worrying.

@dwalton76
Copy link
Copy Markdown
Collaborator

I haven't tested this on-device yet. @WasabiFan have you had a chance to test it on a device? If so it looks good to me.

@WasabiFan
Copy link
Copy Markdown
Member Author

@dwalton76 sorry, I'm quite busy right now so might not be able to take another look at these PRs for another few weeks. If I do get some time I'll report back.

@WasabiFan
Copy link
Copy Markdown
Member Author

WasabiFan commented Oct 27, 2017

I got a chance to test this on my EV3. I instantiated a motor with motors connected and without and it operated as expected.

@dwalton76 dwalton76 merged commit 923080b into develop Nov 14, 2017
@WasabiFan WasabiFan deleted the remove-connected-attribute branch February 14, 2018 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants