Improve HTTP client robustness and remove raw network tests

firmware/http.cc

@@ -117,20 +117,36 @@ static inline void write_request(const char* server, uint16_t port,
       "Range: %s\r\n"
       "\r\n",
       path, server, range_value);
+
+#ifdef DEBUG
+  Serial.println(request_buffer);
+#endif
+
   client->write((const uint8_t*)request_buffer, request_size);
 }
 
 static inline bool read_response_headers(HeaderData* header_data) {
-  int num_read = -1;
+  int num_read = 0;
   while (num_read <= MIN_RESPONSE_LENGTH && client->connected()) {
-    num_read = client->read((uint8_t*)response_buffer, sizeof(response_buffer));
+    // FIXME: ensure all headers are buffered here
+    int last_read = client->read(
+        (uint8_t*)response_buffer + num_read,
+        sizeof(response_buffer) - num_read);
+    if (last_read >= 0) {
+      num_read += last_read;
+    }
   }
+  response_buffer[num_read] = '\0';
 
   if (num_read < MIN_RESPONSE_LENGTH) {
     Serial.println("Failed!  Did not find status code!");
     return false;
   }
 
+#ifdef DEBUG
+  Serial.println(response_buffer);
+#endif
+
   char status_code_buf[4];
   memcpy(status_code_buf, response_buffer + HTTP_RESPONSE_HEADER_LENGTH, 3);
   status_code_buf[3] = '\0';
@@ -150,19 +166,12 @@ static inline bool read_response_headers(HeaderData* header_data) {
   int body_bytes_in_first_buffer = 0;
 
   // Scan through the buffer looking for the body length and the end of the
-  // headers.
-  for (int i = 0; i < num_read - 3; ++i) {
-    if (response_buffer[i    ] == '\r' &&
-        response_buffer[i + 1] == '\n' &&
-        response_buffer[i + 2] == '\r' &&
-        response_buffer[i + 3] == '\n') {
-      // End of headers.  Save the location and length of the body bytes we
-      // have in buffer, then quit the loop.
-      header_data->body_start = (const uint8_t*)response_buffer + i + 4;
-      header_data->body_start_length = num_read - (i + 4);
-      break;
-    } else if (response_buffer[i] == '\r' &&
-               response_buffer[i + 1] == '\n') {
+  // headers.  NOTE: Although compliant servers are supposed to send \r\n as a
+  // line terminator, compliant clients may ignore the \r and accept \n only.
+  // Android's com.phlox.simpleserver, which I used briefly during testing,
+  // only replies with \n, so we take care here to tolerate that.
+  for (int i = 0; i < num_read - 1; ++i) {
+    if (response_buffer[i] == '\n') {
       if (this_header_starts > 0) {
         // Parse one more header, looking for Content-Length.
         if (!strncasecmp(
@@ -176,7 +185,27 @@ static inline bool read_response_headers(HeaderData* header_data) {
         }
       }
 
-      this_header_starts = i + 2;
+      this_header_starts = i + 1;
+    }
+
+    if (i < num_read - 3 &&
+        response_buffer[i    ] == '\r' &&
+        response_buffer[i + 1] == '\n' &&
+        response_buffer[i + 2] == '\r' &&
+        response_buffer[i + 3] == '\n') {
+      // End of headers (compliant, DOS style line termination).
+      // Save the location and length of the body bytes we have in buffer, then
+      // quit the loop.
+      header_data->body_start = (const uint8_t*)response_buffer + i + 4;
+      header_data->body_start_length = num_read - (i + 4);
+      break;
+    } else if (response_buffer[i    ] == '\n' &&
+               response_buffer[i + 1] == '\n') {
+      // End of headers.  Save the location and length of the body bytes we
+      // have in buffer, then quit the loop.
+      header_data->body_start = (const uint8_t*)response_buffer + i + 2;
+      header_data->body_start_length = num_read - (i + 2);
+      break;
     }
   }
 

firmware/speed-tests.cc

@@ -16,7 +16,7 @@
 #include "sram.h"
 
 #define SERVER "storage.googleapis.com"
-#define RAW_VIDEO_PATH   "/sega-kinetoscope/canned-videos/Never%20Gonna%20Give%20You%20Up.segavideo"
+#define PORT 80
 #define RLE_VIDEO_PATH   "/sega-kinetoscope/canned-videos/Never%20Gonna%20Give%20You%20Up.segavideo.rle"
 
 // 3s chunk of audio+video data, at default settings, without main headers
@@ -96,32 +96,14 @@ extern bool http_rle_sram_callback(const uint8_t* buffer, int bytes);
 extern void http_rle_reset();
 extern bool network_connected;
 
-static long test_raw_download_speed() {
-  // 2.5Mbps minimum required
-  // ~2.7Mbps with initial HTTP connection overhead
-  // ~3.0Mbps on subsequent requests
-  long start = millis();
-  sram_start_bank(0);
-  if (!http_fetch(SERVER,
-                  /* default port */ 0,
-                  RAW_VIDEO_PATH,
-                  /* first byte */ 0,
-                  /* total_size= */ ABOUT_3S_VIDEO_AUDIO_BYTES,
-                  http_sram_callback)) {
-    Serial.println("Fetch failed!");
-  }
-  sram_flush_and_release_bank();
-  long end = millis();
-  return end - start;
-}
-
 static long test_rle_download_speed(int offset, int size) {
   // (Effective) 2.5Mbps minimum required
   // (Effective) ~5.1 Mbps (after decompression)
   http_rle_reset();
   long start = millis();
   sram_start_bank(0);
-  if (!http_fetch(SERVER, /* default port */ 0,
+  if (!http_fetch(SERVER,
+                  PORT,
                   RLE_VIDEO_PATH,
                   offset,
                   size,
@@ -163,21 +145,11 @@ void run_tests() {
   } else {
     Serial.println("Beginning raw network tests.");
 
-    for (int i = 0; i < 10; i++) {
-      ms = test_raw_download_speed();
-      float bits = ABOUT_3S_VIDEO_AUDIO_BYTES * 8.0;
-      float seconds = ms / 1000.0;
-      float mbps = bits / seconds / 1024.0 / 1024.0;
-      Serial.print(ms);
-      Serial.print(" ms to stream ~3s raw video to SRAM (");
-      Serial.print(mbps);
-      Serial.println(" Mbps vs 2.50 Mbps minimum)");
-    }
-
     Serial.println("Beginning RLE network tests.");
     uint32_t minimal_index[2];
     http_local_buffer = (uint8_t*)minimal_index;
-    if (!http_fetch(SERVER, /* default port */ 0,
+    if (!http_fetch(SERVER,
+                    PORT,
                     RLE_VIDEO_PATH,
                     sizeof(SegaVideoHeader),
                     sizeof(minimal_index),