Code review

Author: igrr

hardware/esp8266com/esp8266/bootloaders/eboot/Makefile

@@ -7,7 +7,7 @@ TARGET_DIR := ./
 TARGET_OBJ_FILES := \
 	eboot.o \
 	eboot_command.o \
-	flash.o \
+	
 
 TARGET_OBJ_PATHS := $(addprefix $(TARGET_DIR)/,$(TARGET_OBJ_FILES))
 

hardware/esp8266com/esp8266/bootloaders/eboot/eboot.c

@@ -125,7 +125,7 @@ void main()
     if (cmd.action == ACTION_COPY_RAW) {
         ets_putc('c'); ets_putc('p'); ets_putc(':');
         res = copy_raw(cmd.args[0], cmd.args[1], cmd.args[2]);
-        ets_putc((char)0x30+res); ets_putc('\n');
+        ets_putc('0'+res); ets_putc('\n');
         if (res == 0) {
             cmd.action = ACTION_LOAD_APP;
             cmd.args[0] = cmd.args[1];
@@ -136,7 +136,7 @@ void main()
       ets_putc('l'); ets_putc('d'); ets_putc('\n');
       res = load_app_from_flash_raw(cmd.args[0]);
       //we will get to this only on load fail
-      ets_putc('e'); ets_putc(':'); ets_putc((char)0x30+res); ets_putc('\n');
+      ets_putc('e'); ets_putc(':'); ets_putc('0'+res); ets_putc('\n');
     }
 
     if (res) {

hardware/esp8266com/esp8266/bootloaders/eboot/eboot.elf

@@ -4,7 +4,7 @@
  * Redistribution and use is permitted according to the conditions of the
  * 3-clause BSD license to be found in the LICENSE file.
  */
-/*
+
 #include <stddef.h>
 #include <stdint.h>
 #include <stdbool.h>
@@ -46,4 +46,4 @@ int SPIEraseAreaEx(const uint32_t start, const uint32_t size)
 
     return 0;
 }
-*/
+

hardware/esp8266com/esp8266/bootloaders/eboot/flash.c

@@ -12,7 +12,7 @@ int SPIEraseBlock(uint32_t block);
 int SPIEraseSector(uint32_t sector);
 int SPIRead(uint32_t addr, void *dest, size_t size);
 int SPIWrite(uint32_t addr, void *src, size_t size);
-//int SPIEraseAreaEx(const uint32_t start, const uint32_t size);
+int SPIEraseAreaEx(const uint32_t start, const uint32_t size);
 
 #define FLASH_SECTOR_SIZE 0x1000
 #define FLASH_BLOCK_SIZE 0x10000

hardware/esp8266com/esp8266/bootloaders/eboot/flash.h

@@ -350,7 +350,7 @@ uint32_t EspClass::getFreeSketchSpace() {
     uint32_t usedSize = getSketchSize();
     // round one sector up
     uint32_t freeSpaceStart = (usedSize + FLASH_SECTOR_SIZE - 1) & (~(FLASH_SECTOR_SIZE - 1));
-    uint32_t freeSpaceEnd = (uint32_t)&_SPIFFS_start - 0x40200000 - (5 * FLASH_SECTOR_SIZE);
+    uint32_t freeSpaceEnd = (uint32_t)&_SPIFFS_start - 0x40200000;
 
 #ifdef DEBUG_SERIAL
     DEBUG_SERIAL.printf("usedSize=%u freeSpaceStart=%u freeSpaceEnd=%u\r\n", usedSize, freeSpaceStart, freeSpaceEnd);

hardware/esp8266com/esp8266/cores/esp8266/Esp.cpp

@@ -1,14 +1,30 @@
 #include "Updater.h"
 #include "Arduino.h"
 #include "eboot_command.h"
-extern "C"{
-  #include "mem.h"
-}
+
 //#define DEBUG_UPDATER Serial
 
 extern "C" uint32_t _SPIFFS_start;
 
-UpdaterClass::UpdaterClass() : _error(0), _buffer(0), _bufferLen(0), _size(0), _startAddress(0), _currentAddress(0) {}
+UpdaterClass::UpdaterClass() 
+: _error(0)
+, _buffer(0)
+, _bufferLen(0)
+, _size(0)
+, _startAddress(0)
+, _currentAddress(0) 
+{
+}
+
+void UpdaterClass::_reset() {
+  if (_buffer)
+    delete[] _buffer;
+  _buffer = 0;
+  _bufferLen = 0;
+  _startAddress = 0;
+  _currentAddress = 0;
+  _size = 0;
+}
 
 bool UpdaterClass::begin(size_t size){
   if(_size > 0){
@@ -26,17 +42,13 @@ bool UpdaterClass::begin(size_t size){
     return false;
   }
 
-  if(_buffer) os_free(_buffer);
-  _bufferLen = 0;
-  _startAddress = 0;
-  _currentAddress = 0;
-  _size = 0;
+  _reset();
   _error = 0;
 
   //size of current sketch rounded to a sector
   uint32_t currentSketchSize = (ESP.getSketchSize() + FLASH_SECTOR_SIZE - 1) & (~(FLASH_SECTOR_SIZE - 1));
-  //address of the end of the space available for sketch and update (5 sectors are for EEPROM and init data)
-  uint32_t updateEndAddress = (uint32_t)&_SPIFFS_start - 0x40200000 - (5 * FLASH_SECTOR_SIZE);
+  //address of the end of the space available for sketch and update
+  uint32_t updateEndAddress = (uint32_t)&_SPIFFS_start - 0x40200000;
   //size of the update rounded to a sector
   uint32_t roundedSize = (size + FLASH_SECTOR_SIZE - 1) & (~(FLASH_SECTOR_SIZE - 1));
   //address where we will start writing the update
@@ -55,7 +67,7 @@ bool UpdaterClass::begin(size_t size){
   _startAddress = updateStartAddress;
   _currentAddress = _startAddress;
   _size = size;
-  _buffer = (uint8_t*)os_malloc(FLASH_SECTOR_SIZE);
+  _buffer = new uint8_t[FLASH_SECTOR_SIZE];
 
   return true;
 }
@@ -72,11 +84,8 @@ bool UpdaterClass::end(bool evenIfRemaining){
 #ifdef DEBUG_UPDATER
     DEBUG_UPDATER.printf("premature end: res:%u, pos:%u/%u\n", getError(), progress(), _size);
 #endif
-    if(_buffer) os_free(_buffer);
-    _bufferLen = 0;
-    _currentAddress = 0;
-    _startAddress = 0;
-    _size = 0;
+    
+    _reset();
     return false;
   }
 
@@ -86,9 +95,6 @@ bool UpdaterClass::end(bool evenIfRemaining){
     }
     _size = progress();
   }
-  if(_buffer) os_free(_buffer);
-  _bufferLen = 0;
-  _currentAddress = 0;
 
   eboot_command ebcmd;
   ebcmd.action = ACTION_COPY_RAW;
@@ -100,19 +106,19 @@ bool UpdaterClass::end(bool evenIfRemaining){
 #ifdef DEBUG_UPDATER
     DEBUG_UPDATER.printf("Staged: address:0x%08X, size:0x%08X\n", _startAddress, _size);
 #endif
-  
-  _startAddress = 0;
-  _size = 0;
-  _error = UPDATE_ERROR_OK;
+
+  _reset();
   return true;
 }
 
 bool UpdaterClass::_writeBuffer(){
   noInterrupts();
   int rc = SPIEraseSector(_currentAddress/FLASH_SECTOR_SIZE);
-  if(!rc) rc = SPIWrite(_currentAddress, _buffer, _bufferLen);
+  if (!rc) {
+    rc = SPIWrite(_currentAddress, _buffer, _bufferLen);
+  }
   interrupts();
-  if (rc){
+  if (rc) {
     _error = UPDATE_ERROR_WRITE;
     _currentAddress = (_startAddress + _size);
 #ifdef DEBUG_UPDATER
@@ -125,15 +131,15 @@ bool UpdaterClass::_writeBuffer(){
   return true;
 }
 
-size_t UpdaterClass::write(uint8_t *data, size_t len){
+size_t UpdaterClass::write(uint8_t *data, size_t len) {
   size_t left = len;
-  if(hasError()||!isRunning())
+  if(hasError() || !isRunning())
     return 0;
 
   if(len > remaining())
     len = remaining();
 
-  while((_bufferLen + left) > FLASH_SECTOR_SIZE){
+  while((_bufferLen + left) > FLASH_SECTOR_SIZE) {
     size_t toBuff = FLASH_SECTOR_SIZE - _bufferLen;
     memcpy(_buffer + _bufferLen, data + (len - left), toBuff);
     _bufferLen += toBuff;
@@ -155,13 +161,13 @@ size_t UpdaterClass::write(uint8_t *data, size_t len){
   return len;
 }
 
-size_t UpdaterClass::writeStream(Stream &data){
+size_t UpdaterClass::writeStream(Stream &data) {
   size_t written = 0;
   size_t toRead = 0;
-  if(hasError()||!isRunning())
+  if(hasError() || !isRunning())
     return 0;
 
-  while(remaining()){
+  while(remaining()) {
     toRead = FLASH_SECTOR_SIZE - _bufferLen;
     toRead = data.readBytes(_buffer + _bufferLen, toRead);
     if(toRead == 0){ //Timeout

hardware/esp8266com/esp8266/cores/esp8266/Updater.cpp

@@ -27,7 +27,7 @@ class UpdaterClass {
     size_t write(uint8_t *data, size_t len);
 
     /*
-      Writes the remaining bytes from the Sream to the flash
+      Writes the remaining bytes from the Stream to the flash
       Uses readBytes() and sets UPDATE_ERROR_STREAM on timeout
       Returns the bytes written
       Should be equal to the remaining bytes when called
@@ -53,32 +53,33 @@ class UpdaterClass {
     void printError(Stream &out);
 
     //Helpers
-    inline uint8_t getError(){ return _error; }
-    inline void clearError(){ _error = UPDATE_ERROR_OK; }
-    inline bool hasError(){ return _error != UPDATE_ERROR_OK; }
-    inline bool isRunning(){ return _size > 0; }
-    inline bool isFinished(){ return _currentAddress == (_startAddress + _size); }
-    inline size_t size(){ return _size; }
-    inline size_t progress(){ return _currentAddress - _startAddress; }
-    inline size_t remaining(){ return _size - (_currentAddress - _startAddress); }
+    uint8_t getError(){ return _error; }
+    void clearError(){ _error = UPDATE_ERROR_OK; }
+    bool hasError(){ return _error != UPDATE_ERROR_OK; }
+    bool isRunning(){ return _size > 0; }
+    bool isFinished(){ return _currentAddress == (_startAddress + _size); }
+    size_t size(){ return _size; }
+    size_t progress(){ return _currentAddress - _startAddress; }
+    size_t remaining(){ return _size - (_currentAddress - _startAddress); }
 
     /*
       Template to write from objects that expose
       available() and read(uint8_t*, size_t) methods
-      faster than the readStream method
+      faster than the writeStream method
       writes only what is available
     */
     template<typename T>
     size_t write(T &data){
       size_t written = 0;
-      if(hasError()||!isRunning())
+      if (hasError() || !isRunning())
         return 0;
+
       size_t available = data.available();
-      while(available){
-        if((_bufferLen + available) > remaining()){
-          available = (remaining() - _bufferLen);
+      while(available) {
+        if(_bufferLen + available > remaining()){
+          available = remaining() - _bufferLen;
         }
-        if((_bufferLen + available) > FLASH_SECTOR_SIZE){
+        if(_bufferLen + available > FLASH_SECTOR_SIZE) {
           size_t toBuff = FLASH_SECTOR_SIZE - _bufferLen;
           data.read(_buffer + _bufferLen, toBuff);
           _bufferLen += toBuff;
@@ -89,8 +90,8 @@ class UpdaterClass {
           data.read(_buffer + _bufferLen, available);
           _bufferLen += available;
           written += available;
-          if(_bufferLen == remaining()){
-            if(!_writeBuffer()){
+          if(_bufferLen == remaining()) {
+            if(!_writeBuffer()) {
               return written;
             }
           }
@@ -104,14 +105,15 @@ class UpdaterClass {
     }
 
   private:
+    void _reset();
+    bool _writeBuffer();
+
     uint8_t *_buffer;
-    uint8_t _error;
     size_t _bufferLen;
     size_t _size;
     uint32_t _startAddress;
     uint32_t _currentAddress;
-    
-    bool _writeBuffer();
+    uint8_t _error;
 };
 
 extern UpdaterClass Update;