New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Survey module #22

Merged
merged 45 commits into from Jun 28, 2016

Conversation

Projects
None yet
3 participants
@CdavM
Contributor

CdavM commented Jun 3, 2016

This PR should be used to discuss the implementation of the survey module, which will collect data about node SNR, neighbors and channel allocations.

@@ -452,6 +452,7 @@ def user_url(user):
'nodewatcher.modules.monitor.http.resources.processors.SystemStatus',
'nodewatcher.modules.monitor.http.interfaces.processors.DatastreamInterfaces',
'nodewatcher.modules.monitor.http.clients.processors.ClientInfo',
'nodewatcher.modules.monitor.http.survey.processors.SurveyInfo',

This comment has been minimized.

@CdavM

CdavM Jun 3, 2016

Contributor

Do I have to add this line into MONITOR_RUNS processors list as well?

@CdavM

CdavM Jun 3, 2016

Contributor

Do I have to add this line into MONITOR_RUNS processors list as well?

This comment has been minimized.

@kostko

kostko Jun 4, 2016

Member

It is already included in MONITOR_RUNS, check the telemetry and telemetry-push run definitions, which reference TELEMETRY_PROCESSOR_PIPELINE.

@kostko

kostko Jun 4, 2016

Member

It is already included in MONITOR_RUNS, check the telemetry and telemetry-push run definitions, which reference TELEMETRY_PROCESSOR_PIPELINE.

@CdavM CdavM changed the title from Implemented the survey module to Implemented the survey module. Jun 3, 2016

@kostko kostko changed the title from Implemented the survey module. to Survey module Jun 4, 2016

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Jun 5, 2016

Member

Please use full sentences for commit messages. (Dot at the end.)

Member

mitar commented Jun 5, 2016

Please use full sentences for commit messages. (Dot at the end.)

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Jun 5, 2016

Member

So currently you are not storing survey data into the database. You probably want more than just count of neighbors to compute your stuff, no?

Member

mitar commented Jun 5, 2016

So currently you are not storing survey data into the database. You probably want more than just count of neighbors to compute your stuff, no?

@kostko

This comment has been minimized.

Show comment
Hide comment
@kostko
Member

kostko commented Jun 6, 2016

vertices.append(source_vertex)
for radio in context.http.core.wireless.radios.values():
for neighbor in radio.survey:
vertices.append({'i': neighbor['bssid']})

This comment has been minimized.

@mitar

mitar Jun 21, 2016

Member

What if the same BSSID is present multiple times. Is this possible? Virtual SSIDs?

@mitar

mitar Jun 21, 2016

Member

What if the same BSSID is present multiple times. Is this possible? Virtual SSIDs?

This comment has been minimized.

@CdavM

CdavM Jun 21, 2016

Contributor

The way I understand it, a BSSID is supposed to be unique (unlike a (virtual) SSID that multiple BSSIDs can share). But you identify a particular WAP through its BSSID, just like you identify a computer on the network by its MAC. So while you can spoof the BSSID (or MAC address) I don't know if you want to build in some protection against that? Do you want me to check if one exists before adding it?

@CdavM

CdavM Jun 21, 2016

Contributor

The way I understand it, a BSSID is supposed to be unique (unlike a (virtual) SSID that multiple BSSIDs can share). But you identify a particular WAP through its BSSID, just like you identify a computer on the network by its MAC. So while you can spoof the BSSID (or MAC address) I don't know if you want to build in some protection against that? Do you want me to check if one exists before adding it?

This comment has been minimized.

@mitar

mitar Jun 21, 2016

Member

But if the network is on 2.4 GHz and 5 GHz, does it have to have different BSSID?

@mitar

mitar Jun 21, 2016

Member

But if the network is on 2.4 GHz and 5 GHz, does it have to have different BSSID?

This comment has been minimized.

@CdavM

CdavM Jun 21, 2016

Contributor

My understanding is that you need two antennas to broadcast on two different frequencies. And each antenna has a BSSID associated with it. Otherwise your computer couldn't distinguish between connecting to the 2.4 or 5GHz band? For instance, look at the interfaces at https://github.com/CdavM/swoon/blob/master/test_data/10.20.32.40.feed
wlan0 (5GHz) has a bssid of 04:18:D6:21:60:44 and wlan1 (2.4GHz) has a bssid of 04:18:D6:22:60:44

@CdavM

CdavM Jun 21, 2016

Contributor

My understanding is that you need two antennas to broadcast on two different frequencies. And each antenna has a BSSID associated with it. Otherwise your computer couldn't distinguish between connecting to the 2.4 or 5GHz band? For instance, look at the interfaces at https://github.com/CdavM/swoon/blob/master/test_data/10.20.32.40.feed
wlan0 (5GHz) has a bssid of 04:18:D6:21:60:44 and wlan1 (2.4GHz) has a bssid of 04:18:D6:22:60:44

This comment has been minimized.

@CdavM

CdavM Jun 21, 2016

Contributor

So this means we only have one edge between nodes in our graph but this should be fine.

@CdavM

CdavM Jun 21, 2016

Contributor

So this means we only have one edge between nodes in our graph but this should be fine.

This comment has been minimized.

@mitar

mitar Jun 21, 2016

Member

OK. Sounds good.

@mitar

mitar Jun 21, 2016

Member

OK. Sounds good.

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Jun 21, 2016

Member

I think this is good now. @kostko, what do you think? Ready to merge?

Member

mitar commented Jun 21, 2016

I think this is good now. @kostko, what do you think? Ready to merge?

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Jun 21, 2016

Member

BTW, you are writing documentation on how to write nodewatcher modules in meantime? :-)

Member

mitar commented Jun 21, 2016

BTW, you are writing documentation on how to write nodewatcher modules in meantime? :-)

@kostko

This comment has been minimized.

Show comment
Hide comment
@kostko

kostko Jun 26, 2016

Member

I think these commits should be squashed into a single commit called "Implement survey module." or something. And rebased on latest development branch. Because most of these are just fixes based on review comments.

Member

kostko commented Jun 26, 2016

I think these commits should be squashed into a single commit called "Implement survey module." or something. And rebased on latest development branch. Because most of these are just fixes based on review comments.

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Jun 26, 2016

Member

Hm, we have never done squashing and rebasing? I am against loosing history. Why we would start now? It does not matter if it is ugly, this was also a process.

But otherwise you are OK with the content of the pull request?

Member

mitar commented Jun 26, 2016

Hm, we have never done squashing and rebasing? I am against loosing history. Why we would start now? It does not matter if it is ugly, this was also a process.

But otherwise you are OK with the content of the pull request?

@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Jun 28, 2016

Member

OK, I am merging it as it is. :-)

Member

mitar commented Jun 28, 2016

OK, I am merging it as it is. :-)

@mitar mitar merged commit a7b5e19 into wlanslovenija:development Jun 28, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mitar

This comment has been minimized.

Show comment
Hide comment
@mitar

mitar Jun 28, 2016

Member

Congrats!

Member

mitar commented Jun 28, 2016

Congrats!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment