Airplane mode feature (redesigned) #888

evergreen22 · 2021-12-24T03:14:20Z

Add airplane mode feature (redesigned)

Implement 'Airplane mode' to disable and enable bluetooth
Add a setting menu for switching airplane mode on and off
Add an airplane icon that is displayed when airplane mode is enabled

You turn on airplane mode by swiping and selecting settings, selecting Airplane mode, then choosing Yes. This will disconnect the watch if it is currently connected or cancel advertising if it is not connected. When airplane mode is on you cannot connect to the watch (i.e. bluetooth communications are disabled).

To turn airplane mode off, swipe and select settings, select Airplane mode, and choose No. If your companion app is set to automatically reconnect then your watch will automatically connect.

Note that in both cases, turning airplane mode on and off, the change happens when you close the Airplane mode setting menu (swipe down).

The airplane mode setting is not saved. Rebooting the watch will turn off airplane mode.

In addition, the following style and organizational changes are included in PR:

Alphabetize the settings menu options
Style cleanups

Resolves #632

Implements 'Airplane mode' feature to disable and enable bluetooth/ble Adds airplaneMode as a non-persisted setting Adds a setting menu for switching airplane mode on and off Displays an airplane symbol on the Digital watch face and the PineTimeStyle watch face when airplane mode is enabled Always enables bluetooth/ble on boot (disable airplane mode) Alphabetizes the settings menu options Style cleanups Closes InfiniTimeOrg#632

trman · 2021-12-30T21:08:31Z

hi ,

i have tested the pr and it work great !

the setting order is pretty disturbing at first but become intuitive after two day or so .

great redesign @evergreen22 !

i wish that it would be merged by @JF002 for 1.8.0 too !

geekbozu · 2022-01-01T16:55:07Z

Code looks clean,
Do you have the size increases associated with this?

trman · 2022-01-01T16:57:48Z

Code looks clean, Do you have the size increases associated with this?

for what i see in the pr action @geekbozu , compared to last dev action github , i'ts around 1k more (maybe related to the new icon )
but the icon is necessary so it's not a bad thing

Itai-Nelken · 2022-01-03T16:02:31Z

While I think the settings order isn't the best, alphabetizing it is worse.
For example: now 'About' is at the top, while it is one of the least used entries, and anyway has a shortcut (very long press of the button). so it should be at the bottom.
Maybe reordering the settings should be in another PR?

That is my opinion as a mostly regular user.

Riksu9000

Code looks good and seems to work as expected. I can't test if the watch leaks any signals, but it disconnects and can't be found when scanning.

It would be much better to have this option in the quick settings, though that's obviously not possible yet. We should look into reworking that so it can hold more functionalities as they come.

Riksu9000 · 2022-01-03T16:30:29Z

src/displayapp/screens/settings/SettingAirplaneMode.h

+        void UpdateSelected(lv_obj_t* object, lv_event_t event);
+
+      private:
+        static constexpr std::array<const char*, 2> options = {" No", " Yes"};


I think the "Yes" option should be above "No"

EDIT: Perhaps call it Enabled and Disabled instead.

I disagree.

I did a HCI/UX paper prototype study to test the following options before selecting this option (n=16):

Yes/No 56% of respondents performed correctly

On/Off 25% of respondents performed correctly

Enable/Disable 19% of respondents performed correctly

Regarding the order, the existing UX displays the default choice first and "No airplane mode" is the default mode. Therefore the order should be "No" then "Yes" for consistency.

I don't really understand what you mean. Do you mean that if a person goes to enable the airplane mode, only 19% would be able to press the correct button to enable it? I highly doubt that is the case. "Airplane mode. Yes/No" doesn't really make sense in a literal sense either.

Having the default option first sounds reasonable, but somehow having "No" above "Yes" doesn't feel right. Unfortunately I'm not able to explain why.

This could also just be a switch widget.

Riksu9000 · 2022-01-03T16:33:37Z

src/components/ble/NimbleController.cpp

+void NimbleController::SwitchAirplaneMode(bool enabled) {
+  if (enabled) {
+    if (bleController.IsConnected()) {
+      bleController.SetConnectState(Ble::ConnectStates::Airplane);
+      ble_gap_terminate(connectionHandle, BLE_ERR_REM_USER_CONN_TERM);
+    } else {
+      bleController.SetConnectState(Ble::ConnectStates::Airplane);
+      ble_gap_adv_stop();
+    }
+  } else {
+    bleController.SetConnectState(Ble::ConnectStates::Disconnected);
+    fastAdvCount = 0;
+    StartAdvertising();
+  }
+}
+


I would rather have this be two functions without a boolean parameter.

I disagree. I see no reason to make the code larger.

Riksu9000 · 2022-01-03T16:35:59Z

src/displayapp/screens/settings/Settings.cpp

-    {Symbols::sun, "Display", Apps::SettingDisplay},
-    {Symbols::eye, "Wake Up", Apps::SettingWakeUp},
-    {Symbols::clock, "Time format", Apps::SettingTimeFormat},
-    {Symbols::home, "Watch face", Apps::SettingWatchFace},
+    {Symbols::list, "About", Apps::SysInfo},
+    {Symbols::airplane, "Airplane mode", Apps::SettingAirplaneMode},
+    {Symbols::batteryHalf, "Battery", Apps::BatteryInfo},
+    {Symbols::sun, "Display", Apps::SettingDisplay}


The order shouldn't be alphabetized and shouldn't be changed in this PR.

I agree with @Riksu9000, I'm not sure the alphabetized order is the best one, and I would rather like to see this change in a dedicated PR.

I disagree. While I concede this may not be an ordering that makes 100% of people happy (does that even exist?), I feel strongly that the aribtrary order is wrong from a UX standpoint because it creates an unecessary cognitive burden on the user.

If we order the options alphabetically now I expect it will kick off a serious, evidence based discussion on how this is best resolved. Folks posting about which option they use the most (FWIW I use "About" the most) is not moving the ball forward and just adding new options in random spots is procrastination.

At the very least, alphabetical ordering is a common, accepted UX pattern for options that is widely understood.

I agree that alphabetical ordering has its benefits. However when changing languages, should the order change according to the used language? Alphabetical ordering isn't common for settings on other platforms either. At the very least this will not be changed in a PR about airplane mode.

JF002

Thanks for this nice and complete PR (new feature + settings + icon, perfect!).

Could you have a look at the @Riksu9000's and mine reviews?

JF002 · 2022-01-31T20:14:24Z

src/components/ble/BleController.cpp

@@ -2,12 +2,12 @@

 using namespace Pinetime::Controllers;

-void Ble::Connect() {
-  isConnected = true;
+void Ble::SetConnectState(Ble::ConnectStates newState) {


I'm not sure we should mix "connection state" (connected and disconnected are implemented, and other future states could be "error", "failed", "refused",...) with an "operating mode" (normal and airplane mode).
I think they are 2 distinct concept that should be kept separate :

ConnectionMode : What is the status of the BLE connectivity : are we connected or not ?

OperatingMode (or ConnectionAllowed, or something similar?) : Do we accept new BLE connections ?

What do you think?

JF002 · 2022-01-31T20:16:06Z

src/components/ble/NimbleController.h

      void Init();
      void StartAdvertising();
      int OnGAPEvent(ble_gap_event* event);

+      /* these are not implemented yet


You can safely remove those declarations of the methods are not implemented in NimbleController.cpp.

I didn't remove them because I wasn't sure if they had any value from a documentation standpoint.

I think I forgot to remove them a long time ago. They are not needed since these functionalities are implemented is services.

JF002 · 2022-01-31T20:21:00Z

@evergreen22 I have one question regarding this PR : If I understand correctly the changes, the radio is not "physically" switched off. Do you know if there's any chance the radio is still emitting signal when the airplane mode is enabled?
I think this PR is still valid even if it does, but I think this would be good to know!

Also, what prevents a bonded device from re-connecting to the watch, as a bonded device does not need an advertising packet to be able to connect to the device ?

JF002 · 2022-02-20T14:44:27Z

I applied the changes I asked for and merged the PR. Thanks @evergreen22 and everyone who contributed to this PR!

WhyNotHugo · 2022-04-01T19:46:45Z

Why is this called "airplane mode"? The watch only has bluetooth, which is nowadays allowed in flights and also actively used. The watch has no cellular antenna either, which does need to be disabled in planes.

I relate deactivating bluetooth more with privacy and battery saving, TBH.

Anyway, I wanted to bring up #1060, I think there's an overlap in functionality between this PR and that feature request.

Mostly, I'd like bluetooth to be turned on after the device boots from an out-of-battery situation. The use case is: I've let it run out of battery and put it in the charging case. When it wakes up, I want it to auto-sync the time with my laptop ASAP, and then turn bluetooth off again. I think this PR is a on/off switch, and I'm mostly looking for an auto setting.

Riksu9000 · 2022-04-02T10:45:00Z

@WhyNotHugo Nobody had thought of that. Renaming it might be a good idea.

Riksu9000 reviewed Jan 3, 2022

View reviewed changes

JF002 added this to the 1.9.0 milestone Jan 19, 2022

JF002 requested changes Jan 31, 2022

View reviewed changes

JF002 merged commit 40cdb54 into InfiniTimeOrg:develop Feb 20, 2022

Elements6007 mentioned this pull request Apr 1, 2022

Keeping bluetooth off #1060

Closed

1 task

Riksu9000 mentioned this pull request Apr 2, 2022

Replace airplane mode with a bluetooth toggle #1065

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Airplane mode feature (redesigned) #888

Airplane mode feature (redesigned) #888

evergreen22 commented Dec 24, 2021

trman commented Dec 30, 2021

geekbozu commented Jan 1, 2022

trman commented Jan 1, 2022 •

edited

Loading

Itai-Nelken commented Jan 3, 2022

Riksu9000 left a comment

Riksu9000 Jan 3, 2022 •

edited

Loading

JF002 Jan 31, 2022

evergreen22 Feb 5, 2022

Riksu9000 Feb 5, 2022

Riksu9000 Jan 3, 2022

evergreen22 Feb 5, 2022

Riksu9000 Jan 3, 2022

JF002 Jan 31, 2022

evergreen22 Feb 5, 2022

Riksu9000 Feb 5, 2022 •

edited

Loading

JF002 left a comment

JF002 Jan 31, 2022

JF002 Jan 31, 2022

evergreen22 Feb 5, 2022

JF002 Feb 6, 2022

JF002 commented Jan 31, 2022

JF002 commented Feb 20, 2022

WhyNotHugo commented Apr 1, 2022

Riksu9000 commented Apr 2, 2022

Airplane mode feature (redesigned) #888

Airplane mode feature (redesigned) #888

Conversation

evergreen22 commented Dec 24, 2021

Add airplane mode feature (redesigned)

trman commented Dec 30, 2021

geekbozu commented Jan 1, 2022

trman commented Jan 1, 2022 • edited Loading

Itai-Nelken commented Jan 3, 2022

Riksu9000 left a comment

Choose a reason for hiding this comment

Riksu9000 Jan 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Riksu9000 Feb 5, 2022 • edited Loading

Choose a reason for hiding this comment

JF002 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JF002 commented Jan 31, 2022

JF002 commented Feb 20, 2022

WhyNotHugo commented Apr 1, 2022

Riksu9000 commented Apr 2, 2022

trman commented Jan 1, 2022 •

edited

Loading

Riksu9000 Jan 3, 2022 •

edited

Loading

Riksu9000 Feb 5, 2022 •

edited

Loading