Skip to content
This repository has been archived by the owner on Aug 11, 2020. It is now read-only.

check if connection is actually established #17

Merged
merged 3 commits into from
Feb 4, 2014
Merged

check if connection is actually established #17

merged 3 commits into from
Feb 4, 2014

Conversation

alexykot
Copy link
Contributor

This checks if connection is actually established and returns assigned IP address if is, or throws ConnectionFail if not.

@rockymeza - yes, please add failing tests as you've proposed and I'll try to figure out how to make it not failing.

@@ -37,3 +37,7 @@ def print_table(matrix, sep=' ', file=sys.stdout, *args, **kwargs):

for row in matrix:
print(format.format(*row).strip(), file=file, *args, **kwargs)


class ConnectionFail(Exception):
Copy link
Owner

Choose a reason for hiding this comment

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

I think I'd rather this live in a different module (maybe wifi.exceptions). I tend to think of utils.py as a file to put stuff that are not related to wifi at all. This ConnectionFail (maybe it could be ConnectionError) is much more related to wifi as a concept that the other functions in this file. Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

since we have two exceptions already - yes, adding separate module makes sense.

@@ -35,6 +36,8 @@ def configuration(cell, passkey=None):
raise NotImplementedError


address_bound_re = re.compile(r'bound to (\d+.\d+.\d+.\d+)')
Copy link
Owner

Choose a reason for hiding this comment

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

Hypothetically I would like to support IPv6... I have no idea what the output would look like, but it might not be all integer values.

I think I better regex would be: r'^bound to (\S+)' or something like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, not sure if \S will match an IPv6 address, as it's usually delimited with colons. Also I have no idea and/or proofs that output for v6 will be same in all other parts outside of the address itself, so not sure if it's worth adjusting it here now.
Yet I can add parsing for generic colon delimited IPv6 address.
What do you think?

Copy link
Owner

Choose a reason for hiding this comment

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

\S matches everything that isn't whitespace.

>>> import re
>>> foo = re.compile(r'^foo (\S+)$')
>>> foo.match('foo 1.2.3.4').group(1)
'1.2.3.4'
>>> foo.match('foo 2001:0000::/29').group(1)
'2001:0000::/29'
>>> foo.match('foo ::/128').group(1)
'::/128'

@rockymeza
Copy link
Owner

Sorry for falling off the map for a while, I had finals... but now I'm finished with them, I can address these this week. We'll get these done and released, sound good?

@alexykot
Copy link
Contributor Author

Sure. What still needs addressing here at your opinion?

@rockymeza
Copy link
Owner

Hey @alexykot, I made some comments regarding the regex. Once those are addressed, this is good to merge.

Before this gets into master, it needs tests, but I can add those if you want. I was thinking about splitting out a function that parses the output of ifup to do that.

@rockymeza rockymeza closed this Jan 16, 2014
@rockymeza rockymeza reopened this Jan 16, 2014
@rockymeza
Copy link
Owner

whoops! sorry.

@rockymeza
Copy link
Owner

I added some tests and the Connection object and merged!!!!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants