Skip to content

My changes to address issues 17, 18, and 19#22

Open
ReyhanJB wants to merge 2 commits intofmetzger:masterfrom
ReyhanJB:master
Open

My changes to address issues 17, 18, and 19#22
ReyhanJB wants to merge 2 commits intofmetzger:masterfrom
ReyhanJB:master

Conversation

@ReyhanJB
Copy link
Copy Markdown

  1. I added broadcast receivers to bluetoothSensor, wifiSensor, and wifiSonnectionSensor classes to listen for changes in the status of bluetooth and wifi that is made by user. The receivers are registered in _enable() and unregistered in _disable(). If the user turns off bluetooth or wifi, the scantask will be stopped (callback is removed). If the user turns on bluetooth or wifi, scantask will be resumed. I intentionally didn't invoke _enable() or _disable() to avoid messing with core functionality of sensors.

  2. I modified implementation of onProviderEnabled and onProviderDisabled for the locationlisteners in NetworkLocationSensor and GPSLocationSensor classes. I have to do a little refactoring (moving initialization of the listeners outside of _enable()) to make things work.

I tested the app to ensure the changes don't break the functionality. Please let me know if things work fine as well on your side.

@aaaaalbert
Copy link
Copy Markdown
Collaborator

@ReyhanJB, thanks for your pull request which I've finally come around to review. There's a couple of issues I see, some functional, some formal:

  • In my humble tests on a Nexus 6 running Android 7.1.1, it doesn't seem that scanTask messages to the debug log actually cease when I turn off Bluetooth or WiFi. (I haven't tested with GPS.) If I remember correctly, these messages brought you to notice issues Scanning for Bluetooth devices even if the Bluetooth is off #17, Stop listening for GPS updates when GPS has been turned off #18, Scanning for wireless/cellular connections even if the wireless/radio is off #19 in the first place.
  • I'd like to ask you to split the commits by topic, and send separate PRs for the three separate issues. This makes reviewing and testing much easier, and thus speeds up getting your code mainlined and released. For instance, perhaps your GPS fixes work as advertised --- I'd still vote to close this current PR because of other unresolved issues, and then your GPS fixes won't get in.
  • On a similar note: removing unneccessary metadata or build files is highly welcome, but cleanup like this should also happen on a separate PR.
  • Commit messages are important communication with other coders, including your future self. Never leave them empty; always strive to write great ones. This is a good guide to stick to.
  • I notice a mix of Unix and DOS line endings in your PR. Please set your editor/IDE to Unix-style (i.e. use \n only), and/or use dos2unix or a similar tool before you commit your code. (This has a very practical reason: Fixing the line endings after the fact introduces noisy yet uninteresting commits, and diff will show tons of almost-identical lines in both the > and < sections).

Feel free to reach out if I should clarify the points above (or anything else, actually). Your contributions are much appreciated!

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.

2 participants