commaai · yuzisee · Nov 20, 2023 · Nov 20, 2023 · Nov 20, 2023 · Nov 20, 2023
diff --git a/common/params.cc b/common/params.cc
@@ -126,7 +126,6 @@ std::unordered_map<std::string, uint32_t> keys = {
     {"GithubUsername", PERSISTENT},
     {"GitRemote", PERSISTENT},
     {"GsmApn", PERSISTENT},
-    {"GsmMetered", PERSISTENT},
     {"GsmRoaming", PERSISTENT},
     {"HardwareSerial", PERSISTENT},
     {"HasAcceptedTerms", PERSISTENT},
@@ -157,6 +156,7 @@ std::unordered_map<std::string, uint32_t> keys = {
     {"LiveParameters", PERSISTENT},
     {"LiveTorqueParameters", PERSISTENT | DONT_LOG},
     {"LongitudinalPersonality", PERSISTENT},
+    {"MeteredUploadPolicy", PERSISTENT},
     {"NavDestination", CLEAR_ON_MANAGER_START | CLEAR_ON_OFFROAD_TRANSITION},
     {"NavDestinationWaypoints", CLEAR_ON_MANAGER_START | CLEAR_ON_OFFROAD_TRANSITION},
     {"NavPastDestinations", PERSISTENT},

diff --git a/selfdrive/athena/athenad.py b/selfdrive/athena/athenad.py
@@ -74,6 +74,45 @@ class UploadFile:
   headers: Dict[str, str]
   allow_cellular: bool
 
+  def upload_on_cellular_allowed_by_params(self) -> bool:
+    # low_upload     is upload_policy=0
+    # default_upload is upload_policy=1
+    # full_upload    is upload_policy=2
+    # see metered_upload_button_texts in selfdrive/ui/qt/network/networking.cc#AdvancedNetworking::AdvancedNetworking
+
+    param_value = Params().get('MeteredUploadPolicy')
+    if param_value is not None:
+      upload_policy = int(param_value)
+    else:
+      upload_policy = 1
+
+    # TODO(from yuzisee): does athenad know whether we are using a Comma SIM (i.e. whether the user is subscribed to full prime?)
+    # e.g.
+    # ```
+    #   if using_comma_sim_so_prevent_full_upload:
+    #     upload_policy = min(1, upload_policy)
+    # ```
+    #
+    # Instead, for now we do limit the selection itself…
+    #  (see meteredSetting in selfdrive/ui/qt/network/networking.cc#AdvancedNetworking::AdvancedNetworking)
+    #  …which is maybe enough
+
+    if upload_policy == 0:
+      # 'low upload' policy is to upload nothing until we're connected to a full connection (e.g. home Wi-Fi)
+      # see also https://discord.com/channels/469524606043160576/819046761287909446/1161366616920051914
+      return False
+    elif upload_policy == 2:
+      # 'full upload' policy is to always allow all uploads
+      return True
+    else:
+      # This is the `default upload` policy. It's believed to use an *average* of 6 GiB/mo or so...
+      # https://discord.com/channels/469524606043160576/819046761287909446/1161369395382202438
+      # https://discord.com/channels/469524606043160576/819046761287909446/1161366312686198905
+      # At the moment this means we fall back to the UploadFile's initial `allow_cellular` value which
+      #  * will upload the qlogs & qcameras while the connection is metered
+      #  * will upload everything if the connection is not metered
+      return self.allow_cellular
+
   @classmethod
   def from_dict(cls, d: dict) -> UploadFile:
     return cls(d.get("fn", ""), d.get("url", ""), d.get("headers", {}), d.get("allow_cellular", False))
@@ -212,17 +251,29 @@ def retry_upload(tid: int, end_event: threading.Event, increase_count: bool = Tr
         break
 
 
+def should_allow_upload_immediately(sm, item: UploadItem, network_metered: bool, network_type: int) -> bool:
+  is_cellular = network_type not in [NetworkType.wifi, NetworkType.ethernet]
+  if is_cellular:
+    return item.allow_cellular
+  else:
+    # e.g. maybe it's a Wi-Fi hotspot but whose underlying cellular connection is being reported as metered
+    return item.allow_cellular or (not network_metered)
+
+
 def cb(sm, item, tid, sz: int, cur: int) -> None:
   # Abort transfer if connection changed to metered after starting upload
   sm.update(0)
   metered = sm['deviceState'].networkMetered
-  if metered and (not item.allow_cellular):
+  network_type = sm['deviceState'].networkType.raw
+  if not should_allow_upload_immediately(sm, item, metered, network_type):
     raise AbortTransferException
 
   cur_upload_items[tid] = replace(item, progress=cur / sz if sz else 1)
 
 
 def upload_handler(end_event: threading.Event) -> None:
+  # `log_handler` and `stats_handler` add to the queues, but don't actually upload anything themselves
+  # all the upload activity happens here in `upload_handler`
   sm = messaging.SubMaster(['deviceState'])
   tid = threading.get_ident()
 
@@ -246,7 +297,7 @@ def upload_handler(end_event: threading.Event) -> None:
       sm.update(0)
       metered = sm['deviceState'].networkMetered
       network_type = sm['deviceState'].networkType.raw
-      if metered and (not item.allow_cellular):
+      if not should_allow_upload_immediately(sm, item, metered, network_type):
         retry_upload(tid, end_event, False)
         continue
 
@@ -419,7 +470,7 @@ def uploadFilesToUrls(files_data: List[UploadFileDict]) -> UploadFilesToUrlRespo
       headers=file.headers,
       created_at=int(time.time() * 1000),
       id=None,
-      allow_cellular=file.allow_cellular,
+      allow_cellular=file.upload_on_cellular_allowed_by_params(),
     )
     upload_id = hashlib.sha1(str(item).encode()).hexdigest()
     item = replace(item, id=upload_id)

diff --git a/selfdrive/manager/manager.py b/selfdrive/manager/manager.py
@@ -41,7 +41,7 @@ def manager_init() -> None:
   default_params: List[Tuple[str, Union[str, bytes]]] = [
     ("CompletedTrainingVersion", "0"),
     ("DisengageOnAccelerator", "0"),
-    ("GsmMetered", "1"),
+    ("MeteredUploadPolicy", "1"),
     ("HasAcceptedTerms", "0"),
     ("LanguageSetting", "main_en"),
     ("OpenpilotEnabledToggle", "1"),

diff --git a/selfdrive/ui/qt/network/networking.cc b/selfdrive/ui/qt/network/networking.cc
@@ -153,7 +153,7 @@ AdvancedNetworking::AdvancedNetworking(QWidget* parent, WifiManager* wifi): QWid
   roamingToggle = new ToggleControl(tr("Enable Roaming"), "", "", roamingEnabled);
   QObject::connect(roamingToggle, &ToggleControl::toggleFlipped, [=](bool state) {
     params.putBool("GsmRoaming", state);
-    wifi->updateGsmSettings(state, QString::fromStdString(params.get("GsmApn")), params.getBool("GsmMetered"));
+    wifi->updateGsmSettings(state, QString::fromStdString(params.get("GsmApn")), params.get("MeteredUploadPolicy") != "2");
   });
   list->addItem(roamingToggle);
 
@@ -168,27 +168,48 @@ AdvancedNetworking::AdvancedNetworking(QWidget* parent, WifiManager* wifi): QWid
     } else {
       params.put("GsmApn", apn.toStdString());
     }
-    wifi->updateGsmSettings(params.getBool("GsmRoaming"), apn, params.getBool("GsmMetered"));
+    wifi->updateGsmSettings(params.getBool("GsmRoaming"), apn, params.get("MeteredUploadPolicy") != "2");
   });
   list->addItem(editApnButton);
 
   // Metered toggle
-  const bool metered = params.getBool("GsmMetered");
-  meteredToggle = new ToggleControl(tr("Cellular Metered"), tr("Prevent large data uploads when on a metered connection"), "", metered);
-  QObject::connect(meteredToggle, &SshToggle::toggleFlipped, [=](bool state) {
-    params.putBool("GsmMetered", state);
-    wifi->updateGsmSettings(params.getBool("GsmRoaming"), QString::fromStdString(params.get("GsmApn")), state);
-  });
-  list->addItem(meteredToggle);
+  int prime_type = uiState()->primeType();
+  if (prime_type == PrimeType::LITE) {
+    // Bring your own SIM, so allow whatever uploads they want
+    std::vector<QString> metered_upload_button_texts{tr("low upload"), tr("default"), tr("full upload")};
+    meteredSetting = new ButtonParamControl("MeteredUploadPolicy", tr("Cellular Metered"),
+                                            tr("Prevent large data uploads when on a metered connection. Choose 'full upload' if you have unlimited cellular data, and "
+                                               "'low upload' if you want to minimize uploads over cellular as much as possible. "
+                                               "The 'default upload' policy is to upload qlog/qcam over cellular, but full logs only once you're back on Wi-Fi."),
+                                            "",
+                                            metered_upload_button_texts);
+  } else {
+    // If they didn't bring their own SIM, presumably the comma SIM is installed. Allow only the lower data-usage choices.
+    std::vector<QString> metered_upload_button_texts{tr("low upload"), tr("default")};
+    meteredSetting = new ButtonParamControl("MeteredUploadPolicy", tr("Cellular Metered"),
+                                            tr("Prevent large data uploads when on a metered connection. "
+                                               "Choose 'low upload' if you want to minimize uploads over cellular as much as possible. "
+                                               "The 'default upload' policy is to upload qlog/qcam over cellular, but full logs only once you're back on Wi-Fi."),
+                                            "",
+                                            metered_upload_button_texts);
+    // TODO(from yuzisee): Does `prime_type == PrimeType::NONE` mean they brought their own SIM, or that they have the Comma SIM but haven't activated it?
+  }
+  // NOTE: We don't have a connect(…) call like the other toggles, because the ButtonParamControl constructor sets that up for us already
+  //       See selfdrive/ui/qt/widgets/controls.h#ButtonParamControl
+  // However, this also means we don't call
+  // ```
+  // wifi->updateGsmSettings(params.getBool("GsmRoaming"), apn, params.get("MeteredUploadPolicy") != "2");
+  // ```
+  // when the setting changes… so for now, we are extra careful inside selfdrive/athena/athenad.py#should_allow_upload_immediately
+  list->addItem(meteredSetting);
 
   // Set initial config
-  wifi->updateGsmSettings(roamingEnabled, QString::fromStdString(params.get("GsmApn")), metered);
+  wifi->updateGsmSettings(roamingEnabled, QString::fromStdString(params.get("GsmApn")), params.get("MeteredUploadPolicy") != "2");
 
   connect(uiState(), &UIState::primeTypeChanged, this, [=](PrimeType prime_type) {
     bool gsmVisible = prime_type == PrimeType::NONE || prime_type == PrimeType::LITE;
     roamingToggle->setVisible(gsmVisible);
     editApnButton->setVisible(gsmVisible);
-    meteredToggle->setVisible(gsmVisible);
   });
 
   main_layout->addWidget(new ScrollView(list, this));

diff --git a/selfdrive/ui/qt/network/networking.h b/selfdrive/ui/qt/network/networking.h
@@ -62,7 +62,7 @@ class AdvancedNetworking : public QWidget {
   ToggleControl* tetheringToggle;
   ToggleControl* roamingToggle;
   ButtonControl* editApnButton;
-  ToggleControl* meteredToggle;
+  ButtonParamControl *meteredSetting;
   WifiManager* wifi = nullptr;
   Params params;
 

diff --git a/selfdrive/ui/translations/main_ar.ts b/selfdrive/ui/translations/main_ar.ts
@@ -63,8 +63,24 @@
         <translation>محدود بالاتصال الخلوي</translation>
     </message>
     <message>
-        <source>Prevent large data uploads when on a metered connection</source>
-        <translation>منع تحميل البيانات الكبيرة عندما يكون الاتصال محدوداً</translation>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;full upload&apos; if you have unlimited cellular data, and &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">منع تحميل البيانات الكبيرة عندما يكون الاتصال محدوداً</translation>
+    </message>
+    <message>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">منع تحميل البيانات الكبيرة عندما يكون الاتصال محدوداً</translation>
+    </message>
+    <message>
+        <source>low upload</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>default</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>full upload</source>
+        <translation type="unfinished"></translation>
     </message>
 </context>
 <context>

diff --git a/selfdrive/ui/translations/main_de.ts b/selfdrive/ui/translations/main_de.ts
@@ -63,8 +63,24 @@
         <translation>Getaktete Verbindung</translation>
     </message>
     <message>
-        <source>Prevent large data uploads when on a metered connection</source>
-        <translation>Hochladen großer Dateien über getaktete Verbindungen unterbinden</translation>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;full upload&apos; if you have unlimited cellular data, and &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">Hochladen großer Dateien über getaktete Verbindungen unterbinden</translation>
+    </message>
+    <message>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">Hochladen großer Dateien über getaktete Verbindungen unterbinden</translation>
+    </message>
+    <message>
+        <source>low upload</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>default</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>full upload</source>
+        <translation type="unfinished"></translation>
     </message>
 </context>
 <context>

diff --git a/selfdrive/ui/translations/main_fr.ts b/selfdrive/ui/translations/main_fr.ts
@@ -63,8 +63,24 @@
         <translation>Connexion cellulaire limitée</translation>
     </message>
     <message>
-        <source>Prevent large data uploads when on a metered connection</source>
-        <translation>Éviter les transferts de données importants sur une connexion limitée</translation>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;full upload&apos; if you have unlimited cellular data, and &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">Éviter les transferts de données importants sur une connexion limitée</translation>
+    </message>
+    <message>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">Éviter les transferts de données importants sur une connexion limitée</translation>
+    </message>
+    <message>
+        <source>low upload</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>default</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>full upload</source>
+        <translation type="unfinished"></translation>
     </message>
 </context>
 <context>

diff --git a/selfdrive/ui/translations/main_ja.ts b/selfdrive/ui/translations/main_ja.ts
@@ -63,8 +63,24 @@
         <translation>従量制通信設定</translation>
     </message>
     <message>
-        <source>Prevent large data uploads when on a metered connection</source>
-        <translation>大量のデータのアップロードを防止します。</translation>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;full upload&apos; if you have unlimited cellular data, and &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">大量のデータのアップロードを防止します。</translation>
+    </message>
+    <message>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">大量のデータのアップロードを防止します。</translation>
+    </message>
+    <message>
+        <source>low upload</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>default</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>full upload</source>
+        <translation type="unfinished"></translation>
     </message>
 </context>
 <context>

diff --git a/selfdrive/ui/translations/main_ko.ts b/selfdrive/ui/translations/main_ko.ts
@@ -63,8 +63,24 @@
         <translation>데이터 요금제</translation>
     </message>
     <message>
-        <source>Prevent large data uploads when on a metered connection</source>
-        <translation>데이터 요금제 연결 시 대용량 데이터 업로드를 방지합니다</translation>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;full upload&apos; if you have unlimited cellular data, and &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">데이터 요금제 연결 시 대용량 데이터 업로드를 방지합니다</translation>
+    </message>
+    <message>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished">데이터 요금제 연결 시 대용량 데이터 업로드를 방지합니다</translation>
+    </message>
+    <message>
+        <source>low upload</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>default</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>full upload</source>
+        <translation type="unfinished"></translation>
     </message>
 </context>
 <context>

diff --git a/selfdrive/ui/translations/main_nl.ts b/selfdrive/ui/translations/main_nl.ts
@@ -63,7 +63,11 @@
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <source>Prevent large data uploads when on a metered connection</source>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;full upload&apos; if you have unlimited cellular data, and &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Prevent large data uploads when on a metered connection. Choose &apos;low upload&apos; if you want to minimize uploads over cellular as much as possible. The &apos;default upload&apos; policy is to upload qlog/qcam over cellular, but full logs only once you&apos;re back on Wi-Fi.</source>
         <translation type="unfinished"></translation>
     </message>
 </context>