Skip to content
Snippets Groups Projects

On API < 29 wait for Wi-Fi p2p broadcast before creating group

Closed On API < 29 wait for Wi-Fi p2p broadcast before creating group
fix-problem-with-wifi-off-on-api-less-than-29 into master
6 unresolved threads
Closed Sebastian requested to merge fix-problem-with-wifi-off-on-api-less-than-29 into master
6 unresolved threads

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Torsten Grote
  • Torsten Grote
    Torsten Grote @grote started a thread on an old version of the diff
    • app/src/main/java/org/briarproject/hotspot/HotspotManager.java
    123 146 }
    124 147
    125 148 void stopWifiP2pHotspot() {
    149 if (SDK_INT < 29)
    150 try {
    151 ctx.unregisterReceiver(receiver);
  • Sebastian added 1 commit

    added 1 commit

    • e419fc12 - On API < 29 wait for Wi-Fi p2p broadcast before creating group

    Compare with previous version

  • Sebastian added 1 commit

    added 1 commit

    • fcb7354e - On API < 29 wait for Wi-Fi p2p broadcast before creating group

    Compare with previous version

  • Sebastian added 1 commit

    added 1 commit

    • f2241fb6 - On API < 29 wait for Wi-Fi p2p broadcast before creating group

    Compare with previous version

  • Sebastian added 1 commit

    added 1 commit

    • 79c2fb3e - On API < 29 wait for Wi-Fi p2p broadcast before creating group

    Compare with previous version

  • Sebastian added 1 commit

    added 1 commit

    • f3785e01 - On API < 29 wait for Wi-Fi p2p broadcast before creating group

    Compare with previous version

  • Sebastian mentioned in merge request !13 (closed)

    mentioned in merge request !13 (closed)

  • Torsten Grote
    Torsten Grote @grote started a thread on an old version of the diff
    • app/src/main/java/org/briarproject/hotspot/HotspotManager.java
    126 @Override
    127 public void onReceive(Context context, Intent intent) {
    128 int state = intent.getIntExtra(EXTRA_WIFI_STATE,
    129 WIFI_P2P_STATE_DISABLED);
    130 if (state == WIFI_P2P_STATE_ENABLED) {
    131 try {
    132 wifiP2pManager.createGroup(channel, HotspotManager.this);
    133 } catch (SecurityException e) {
    134 // this should never happen, because we request permissions before
    135 throw new AssertionError(e);
    136 }
    137 try {
    138 ctx.unregisterReceiver(receiver);
    139 } catch (IllegalArgumentException e) {
    140 // Can happen if the hotspot got stopped in the meantime and
    141 // the receiver already got unregistered.
  • Sebastian added 4 commits

    added 4 commits

    • 4c9b9b8c - Split ConditionManager into version for pre 29 and 29+
    • 12bd2b86 - Check that intent action matches requested events
    • 022174b6 - Add UiThread annotations in HotspotManager
    • 44f55191 - Keep flag whether receiver is registered

    Compare with previous version

    Toggle commit list
  • Sebastian added 4 commits

    added 4 commits

    • 535735a5 - On API < 29 wait for Wi-Fi p2p broadcast before creating group
    • a7123be1 - Check that intent action matches requested events
    • 5997327a - Add UiThread annotations in HotspotManager
    • f52b1dfc - Keep flag whether receiver is registered

    Compare with previous version

    Toggle commit list
  • Torsten Grote
    Torsten Grote @grote started a thread on the diff
    • app/src/main/java/org/briarproject/hotspot/HotspotManager.java
    112 121 }
    113 122 }
    114 123
    124 // Only used on API < 29
    125 private boolean receiverRegistered = false;
  • Torsten Grote
  • Torsten Grote
    Torsten Grote @grote started a thread on the diff
    • app/src/main/java/org/briarproject/hotspot/HotspotManager.java
    135 int state = intent.getIntExtra(EXTRA_WIFI_STATE,
    136 WIFI_P2P_STATE_DISABLED);
    137 if (state == WIFI_P2P_STATE_ENABLED) {
    138 try {
    139 wifiP2pManager.createGroup(channel, HotspotManager.this);
    140 } catch (SecurityException e) {
    141 // this should never happen, because we request permissions before
    142 throw new AssertionError(e);
    143 }
    144 unregisterReceiver();
    145 }
    146 }
    147
    148 };
    149
    150 private void unregisterReceiver() {
  • Torsten Grote
    Torsten Grote @grote started a thread on the diff
    • app/src/main/java/org/briarproject/hotspot/HotspotManager.java
    145 }
    146 }
    147
    148 };
    149
    150 private void unregisterReceiver() {
    151 if (!receiverRegistered)
    152 return;
    153 try {
    154 ctx.unregisterReceiver(receiver);
    155 receiverRegistered = false;
    156 } catch (IllegalArgumentException e) {
    157 // This gets thrown when we try to unregister an already unregistered
    158 // receiver. We do keep a flag whether the receiver is actually
    159 // registered, so it should not happen.
    160 throw new AssertionError(e);
    • Torsten Grote
      Torsten Grote @grote
      Owner

      I wonder if we even need to catch and rethrow here. Could this be removed or does this add any benefit?

    • Sebastian
      Sebastian @sebkur
      Author Developer

      I think the benefit of the current way is that in error log from reports we receive we know right away that we were, at the time of writing the code, assuming that this is a situation that should never happen. While if we just see an IllegalArgumentException we might wonder if we missed something there and haven given some situation a thought yet.

    • Torsten Grote
      Torsten Grote @grote
      Owner

      Not sure that benefit is worth it to be honest. When we receive stack traces, we usually need to check where they happen and why.

    • Please register or sign in to reply
    • Torsten Grote
      Torsten Grote @grote
      Owner

      If we are now hard-relying on the broadcast on API < 29 just because we have not yet seen an issue with higher API levels, then maybe we should always rely on it, not only to make the code simpler, but also because the same issue might exist there?

    • Sebastian
      Sebastian @sebkur
      Author Developer

      I think we could do that. We have that bit of extra time on higher API levels because the user needs to dismiss the WiFi settings panel and that usually seems to be a lot more time that required for the event to come in. But we can't be sure of course. If we use the broadcast, the fragment might move to the next while the panel is still open, which could be a bit strange. We could then try to make this dependent on the combination of both, the broadcast having been received plus the panel being closed. Not sure if this over-complicates the matter.

    • Torsten Grote
      Torsten Grote @grote
      Owner

      Maybe I am misunderstanding something, but isn't the WiFi panel shown as part of the condition checks that we do before trying to start a hotspot?

      If we use the broadcast, the fragment might move to the next while the panel is still open, which could be a bit strange

      Yeah that would be strange, but how could that happen? Shouldn't we start the hotspot only after we know that Wi-Fi is enabled?

    • Sebastian
      Sebastian @sebkur
      Author Developer

      hmm, the user taps the "start hotspot" button, the conditions get checked. Then potientially the panel appears because the Wifi was disabled. The user enables the switch, but then additionally needs to close the panel. Toggling the switch enables the wifi which will also trigger the broadcast. That extra interaction from the user to close the panel is what usually takes longer than the system enabling the wifi.

    • Torsten Grote
      Torsten Grote @grote
      Owner

      Do we get an activity result from that panel that could be used here maybe?

      How is that done currently?

    • Sebastian
      Sebastian @sebkur
      Author Developer

      yes, that's what we do now, we rely on the activity result

    • Sebastian
      Sebastian @sebkur
      Author Developer

      but I think you're right that there might be corner cases where the activity returned but the wifi hasn't been fully activated yet, causing the same problem we are originally addressing here for lower API devices. We haven't seen this yet, it's just a theory that it could happen.

    • Please register or sign in to reply
  • Sebastian added 1 commit

    added 1 commit

    Compare with previous version

  • Sebastian
    Sebastian @sebkur
    Author Developer

    As things become a bit intertwined, I'm closing this and continuing this as a whole in !13 (closed)

  • Sebastian closed

    closed

    • Please register or sign in to reply