Max17055 improvements (#2197)

* fixes

* more fixes

* more fixes

* fix

* more fixes

* fix icon updates

* fix init unk error

* simplify

* improve icon init

* code format

* Update ui_battinfo.cpp

firmware/application/apps/ui_battinfo.cpp

@@ -24,14 +24,13 @@
 
 #include "event_m0.hpp"
 #include "portapack.hpp"
-
+#include "battery.hpp"
 #include <cstring>
 
 using namespace portapack;
 
 namespace ui {
 
-bool BattinfoView::needRun = true;
 void BattinfoView::focus() {
     button_exit.focus();
 }
@@ -53,23 +52,24 @@ void BattinfoView::update_result() {
         return;
     }
     bool uichg = false;
-    battery::BatteryManagement::getBatteryInfo(percent, voltage, current);
+    uint8_t valid_mask = 0;
+    battery::BatteryManagement::getBatteryInfo(valid_mask, percent, voltage, current);
     // update text fields
-    if (percent <= 100)
+    if (percent <= 100 && (valid_mask & battery::BatteryManagement::BATT_VALID_VOLTAGE) == battery::BatteryManagement::BATT_VALID_VOLTAGE)
         text_percent.set(to_string_dec_uint(percent) + " %");
     else
         text_percent.set("UNKNOWN");
-    if (voltage > 1) {
+    if (voltage > 1 && (valid_mask & battery::BatteryManagement::BATT_VALID_VOLTAGE) == battery::BatteryManagement::BATT_VALID_VOLTAGE) {
         text_voltage.set(to_string_decimal(voltage / 1000.0, 3) + " V");
     } else {
         text_voltage.set("UNKNOWN");
     }
-    if (current != 0) {
+    if ((valid_mask & battery::BatteryManagement::BATT_VALID_CURRENT) == battery::BatteryManagement::BATT_VALID_CURRENT) {
         if (labels_opt.hidden()) uichg = true;
         labels_opt.hidden(false);
         text_current.hidden(false);
         text_charge.hidden(false);
-        text_current.set(to_string_decimal(current / 100000.0, 3) + " mA");
+        text_current.set(to_string_dec_int(current) + " mA");
         text_charge.set(current >= 0 ? "Charging" : "Discharging");
         labels_opt.hidden(false);
     } else {
@@ -80,7 +80,7 @@ void BattinfoView::update_result() {
     }
     if (uichg) set_dirty();
     // to update status bar too, send message in behalf of batt manager
-    BatteryStateMessage msg{percent, current >= 0, voltage};
+    BatteryStateMessage msg{valid_mask, percent, current >= 0, voltage};
     EventDispatcher::send_message(msg);
 }
 
@@ -99,13 +99,12 @@ BattinfoView::BattinfoView(NavigationView& nav)
     };
 
     update_result();
-    needRun = true;
+    if (thread == nullptr) thread = chThdCreateFromHeap(NULL, 1024, NORMALPRIO + 10, BattinfoView::static_fn, this);
-    thread = chThdCreateFromHeap(NULL, 512, NORMALPRIO + 10, BattinfoView::static_fn, this);
 }
 
 msg_t BattinfoView::static_fn(void* arg) {
     auto obj = static_cast<BattinfoView*>(arg);
-    while (needRun) {
+    while (!chThdShouldTerminate()) {
         chThdSleepMilliseconds(16);
         obj->on_timer();
     }
@@ -113,7 +112,6 @@ msg_t BattinfoView::static_fn(void* arg) {
 }
 
 BattinfoView::~BattinfoView() {
-    needRun = false;
     if (thread) {
         chThdTerminate(thread);
         chThdWait(thread);

firmware/application/apps/ui_battinfo.hpp

@@ -76,7 +76,6 @@ class BattinfoView : public View {
         {72, 17 * 16, 96, 32},
         "Back"};
     static msg_t static_fn(void* arg);
-    static bool needRun;
     Thread* thread{nullptr};
 };
 

firmware/application/ui_navigation.cpp

@@ -385,11 +385,16 @@ void SystemStatusView::on_battery_details() {
 }
 
 void SystemStatusView::on_battery_data(const BatteryStateMessage* msg) {
+    if (!batt_was_inited) {
+        batt_was_inited = true;
+        refresh();
+        return;
+    }
     if (!pmem::ui_hide_numeric_battery()) {
-        battery_text.set_battery(msg->percent, msg->on_charger);
+        battery_text.set_battery(msg->valid_mask, msg->percent, msg->on_charger);
     }
     if (!pmem::ui_hide_battery_icon()) {
-        battery_icon.set_battery(msg->percent, msg->on_charger);
+        battery_icon.set_battery(msg->valid_mask, msg->percent, msg->on_charger);
     };
 }
 
@@ -410,14 +415,15 @@ void SystemStatusView::refresh() {
 
     if (!pmem::ui_hide_fake_brightness()) status_icons.add(&button_fake_brightness);
     if (battery::BatteryManagement::isDetected()) {
+        batt_was_inited = true;
         uint8_t percent = battery::BatteryManagement::getPercent();
         if (!pmem::ui_hide_battery_icon()) {
             status_icons.add(&battery_icon);
-            battery_text.set_battery(percent, false);  // got an on select, that may pop up the details of the battery.
+            battery_text.set_battery(percent <= 100 ? 1 : 0, percent, false);  // got an on select, that may pop up the details of the battery.
         };
         if (!pmem::ui_hide_numeric_battery()) {
             status_icons.add(&battery_text);
-            battery_text.set_battery(percent, false);
+            battery_text.set_battery(percent <= 100 ? 1 : 0, percent, false);
         }
     }
 

firmware/application/ui_navigation.hpp

@@ -194,7 +194,8 @@ class SystemStatusView : public View {
 
    private:
     static constexpr auto default_title = "";
-    bool batt_info_up = false;  // to prevent show multiple batt info dialog
+    bool batt_was_inited = false;  // if the battery was off on tart, but later turned on.
+    bool batt_info_up = false;     // to prevent show multiple batt info dialog
     NavigationView& nav_;
 
     Rectangle backdrop{

firmware/common/ads1110.cpp

@@ -118,7 +118,7 @@ uint16_t ADS1110::readVoltage() {
     return (uint16_t)voltage;
 }
 
-void ADS1110::getBatteryInfo(uint8_t& batteryPercentage, uint16_t& voltage) {
+void ADS1110::getBatteryInfo(uint8_t& valid_mask, uint8_t& batteryPercentage, uint16_t& voltage) {
     voltage = readVoltage();
 
     // Calculate the remaining battery percentage
@@ -127,6 +127,7 @@ void ADS1110::getBatteryInfo(uint8_t& batteryPercentage, uint16_t& voltage) {
     // Limit the values to the valid range
     batteryPercentage = (batteryPercentage > 100) ? 100 : batteryPercentage;
     // ToDo: if its > 4, then 100%, if < 3 then 0%
+    valid_mask = 1;  // BATT_VALID_VOLTAGE
 }
 
 } /* namespace ads1110 */

firmware/common/ads1110.hpp

@@ -42,7 +42,7 @@ class ADS1110 {
     bool isDetected() const { return detected_; }
 
     uint16_t readVoltage();
-    void getBatteryInfo(uint8_t& batteryPercentage, uint16_t& voltage);
+    void getBatteryInfo(uint8_t& valid_mask, uint8_t& batteryPercentage, uint16_t& voltage);
 
    private:
     I2C& bus;

firmware/common/battery.cpp

@@ -11,7 +11,7 @@ extern I2C portapack::i2c0;
 
 namespace battery {
 
-constexpr uint32_t BATTERY_UPDATE_INTERVAL = 30000;
+constexpr uint32_t BATTERY_UPDATE_INTERVAL = 20000;
 BatteryManagement::BatteryModules BatteryManagement::detected_ = BatteryManagement::BATT_NONE;
 
 ads1110::ADS1110 battery_ads1110{portapack::i2c0, 0x48};
@@ -19,15 +19,18 @@ max17055::MAX17055 battery_max17055{portapack::i2c0, 0x36};
 
 Thread* BatteryManagement::thread = nullptr;
 
-void BatteryManagement::init() {
+void BatteryManagement::detect() {
     // try to detect supported modules
     detected_ = BATT_NONE;
     if (battery_ads1110.detect()) {
         battery_ads1110.init();
         detected_ = BATT_ADS1110;
-    } else if (battery_max17055.detect()) {
+        return;
-        battery_max17055.init();
+    }
+    if (battery_max17055.detect()) {
+        // battery_max17055.init(); //detect will call this on each "re detect"
         detected_ = BATT_MAX17055;
+        return;
     }
 
     // add new supported module detect + init here
@@ -35,25 +38,28 @@ void BatteryManagement::init() {
 #ifdef USE_BATT_EMULATOR
     if (detected_ == BATT_NONE) {
         detected_ = BATT_EMULATOR;
+        return;
     }
 #endif
+}
 
-    if (detected_ != BATT_NONE) {
+void BatteryManagement::init() {
-        // sets timer to query and broadcats this info
+    detect();
-        create_thread();
+    // sets timer to query and broadcats this info
-    }
+    create_thread();
 }
 
 // sets the values, it the currend module supports it.
-bool BatteryManagement::getBatteryInfo(uint8_t& batteryPercentage, uint16_t& voltage, int32_t& current) {
+void BatteryManagement::getBatteryInfo(uint8_t& valid_mask, uint8_t& batteryPercentage, uint16_t& voltage, int32_t& current) {
     if (detected_ == BATT_NONE) {
-        return false;
+        valid_mask = BATT_VALID_NONE;
+        return;
     } else if (detected_ == BATT_ADS1110) {
-        battery_ads1110.getBatteryInfo(batteryPercentage, voltage);
+        battery_ads1110.getBatteryInfo(valid_mask, batteryPercentage, voltage);
-        return true;
+        return;
     } else if (detected_ == BATT_MAX17055) {
-        battery_max17055.getBatteryInfo(batteryPercentage, voltage, current);
+        battery_max17055.getBatteryInfo(valid_mask, batteryPercentage, voltage, current);
-        return true;
+        return;
     }
     // add new module query here
 
@@ -64,12 +70,12 @@ bool BatteryManagement::getBatteryInfo(uint8_t& batteryPercentage, uint16_t& vol
         voltage = rand() % 1000 + 3000;  // mV
         current = rand() % 150;          // mA
         isCharging = rand() % 2;
+        valid_mask = 3;
         return true;
     }
 #endif
 
     (void)current;
-    return false;
 }
 
 uint16_t BatteryManagement::read_register(const uint8_t reg) {
@@ -88,34 +94,42 @@ bool BatteryManagement::write_register(const uint8_t reg, const uint16_t value)
 
 uint8_t BatteryManagement::getPercent() {
     if (detected_ == BATT_NONE) return 102;
+    uint8_t validity = 0;
     uint8_t batteryPercentage = 0;
     uint16_t voltage = 0;
     int32_t current = 0;
-    getBatteryInfo(batteryPercentage, voltage, current);
+    getBatteryInfo(validity, batteryPercentage, voltage, current);
+    if ((validity & BATT_VALID_VOLTAGE) != BATT_VALID_VOLTAGE) return 102;
     return batteryPercentage;
 }
 
 uint16_t BatteryManagement::getVoltage() {
     if (detected_ == BATT_NONE) return 0;
-    if (detected_ == BATT_NONE) return 102;
+    uint8_t validity = 0;
     uint8_t batteryPercentage = 0;
     uint16_t voltage = 0;
     int32_t current = 0;
-    getBatteryInfo(batteryPercentage, voltage, current);
+    getBatteryInfo(validity, batteryPercentage, voltage, current);
+    if ((validity & BATT_VALID_VOLTAGE) != BATT_VALID_VOLTAGE) return 0;
     return voltage;
 }
 
 msg_t BatteryManagement::timer_fn(void* arg) {
     (void)arg;
-    if (!detected_) return 0;
+    uint8_t validity = 0;
     uint8_t batteryPercentage = 102;
     uint16_t voltage = 0;
     int32_t current = 0;
     chThdSleepMilliseconds(1000);  // wait ui for fully load
     while (1) {
-        if (BatteryManagement::getBatteryInfo(batteryPercentage, voltage, current)) {
+        if (!detected_) {
+            detect();  // try to detect it again, it maybe disconnected while pp was powered up
+            chThdSleepMilliseconds(500);
+        }
+        if (detected_) {
+            BatteryManagement::getBatteryInfo(validity, batteryPercentage, voltage, current);
             // send local message
-            BatteryStateMessage msg{batteryPercentage, current >= 0, voltage};
+            BatteryStateMessage msg{validity, batteryPercentage, current >= 0, voltage};
             EventDispatcher::send_message(msg);
         }
         chThdSleepMilliseconds(BATTERY_UPDATE_INTERVAL);

firmware/common/battery.hpp

@@ -35,10 +35,16 @@ class BatteryManagement {
         BATT_MAX17055 = 2,
         BATT_EMULATOR = 254
     };
+    enum BatteryValidMask {
+        BATT_VALID_NONE = 0,
+        BATT_VALID_VOLTAGE = 1,
+        BATT_VALID_CURRENT = 2,
+    };
     static void init();
+    static void detect();
     static bool isDetected() { return detected_ != BATT_NONE; }
     static BatteryModules detectedModule() { return detected_; }
-    static bool getBatteryInfo(uint8_t& batteryPercentage, uint16_t& voltage, int32_t& current);
+    static void getBatteryInfo(uint8_t& valid_mask, uint8_t& batteryPercentage, uint16_t& voltage, int32_t& current);
     static uint16_t getVoltage();
     static uint8_t getPercent();
     static uint16_t read_register(const uint8_t reg);

firmware/common/max17055.cpp

@@ -31,7 +31,8 @@ namespace max17055 {
 void MAX17055::init() {
     if (!detected_) {
         detected_ = detect();
-    } else {
+    }
+    if (detected_) {  // check again if it is detected
         config();
         setHibCFG(0x0000);
 
@@ -81,8 +82,15 @@ bool MAX17055::detect() {
 
     // Get Data from IC
     if (readMultipleRegister(0x00, _MAX17055_Data, 2, false)) {
-        detected_ = true;
+        if (((_MAX17055_Data[0] != 0x00) && (_MAX17055_Data[0] != 0x02)) || (_MAX17055_Data[1] != 0x00)) {
-        return true;
+            // validate result, since i2c gives a bit of power to the ic, and sometimes it sets the init value.
+            // this will return false when the ic is in init state (0x0002), but on the next iteration it'll give the good value
+            if (detected_ == false) {
+                detected_ = true;
+                init();
+            }
+            return true;
+        }
     }
     detected_ = false;
     return false;
@@ -153,10 +161,17 @@ bool MAX17055::writeMultipleRegister(uint8_t reg, const uint8_t* data, uint8_t l
     return false;
 }
 
-void MAX17055::getBatteryInfo(uint8_t& batteryPercentage, uint16_t& voltage, int32_t& current) {
+void MAX17055::getBatteryInfo(uint8_t& valid_mask, uint8_t& batteryPercentage, uint16_t& voltage, int32_t& current) {
-    voltage = averageVoltage();
+    detect();  // need to detect again, since user can disconnect the ic anytime, and that could send garbage causing flickering data.
-    batteryPercentage = stateOfCharge();
+    if (detected_) {
-    current = instantCurrent();
+        voltage = averageVoltage();
+        batteryPercentage = stateOfCharge();
+        current = instantCurrent();
+        valid_mask = 3;  // BATT_VALID_VOLTAGE + CURRENT
+    } else {
+        // let's indicate the data is wrong. ui will handle this by display UNK values.
+        valid_mask = 0;
+    }
 }
 
 bool MAX17055::setEmptyVoltage(uint16_t _Empty_Voltage) {
@@ -553,7 +568,7 @@ int32_t MAX17055::instantCurrent(void) {
     // Convert to signed int16_t (two's complement)
     int32_t _Signed_Raw = static_cast<int16_t>(_Measurement_Raw);
 
-    int32_t _Value = (_Signed_Raw * 15625) / (__MAX17055_Resistor__ * 100);
+    int32_t _Value = (_Signed_Raw * 15625) / (__MAX17055_Resistor__ * 100) / 100000;
 
     // End Function
     return _Value;
@@ -566,7 +581,7 @@ int32_t MAX17055::averageCurrent(void) {
     // Convert to signed int16_t (two's complement)
     int32_t _Signed_Raw = static_cast<int16_t>(_Measurement_Raw);
 
-    int32_t _Value = (_Signed_Raw * 15625) / (__MAX17055_Resistor__ * 100);
+    int32_t _Value = (_Signed_Raw * 15625) / (__MAX17055_Resistor__ * 100) / 100000;
 
     // End Function
     return _Value;

firmware/common/max17055.hpp

@@ -47,7 +47,7 @@
 
 // Define Battery Capacity
 #ifndef __MAX17055_Design_Capacity__
-#define __MAX17055_Design_Capacity__ 1500  // Battery Capacity
+#define __MAX17055_Design_Capacity__ 2500  // Battery Capacity
 #endif
 
 // Define Gauge Resistor
@@ -62,7 +62,7 @@
 
 // Define Maximum Voltage
 #ifndef __MAX17055_Max_Voltage__
-#define __MAX17055_Max_Voltage__ 4.2  // Maximum Voltage
+#define __MAX17055_Max_Voltage__ 4.175  // Maximum Voltage
 #endif
 
 // Define Empty Voltage
@@ -263,7 +263,7 @@ class MAX17055 {
 
     uint16_t readVoltage();
     uint8_t readPercentage();
-    void getBatteryInfo(uint8_t& batteryPercentage, uint16_t& voltage, int32_t& current);
+    void getBatteryInfo(uint8_t& valid_mask, uint8_t& batteryPercentage, uint16_t& voltage, int32_t& current);
 
     uint16_t instantVoltage(void);
     uint16_t averageVoltage(void);
@@ -290,7 +290,7 @@ class MAX17055 {
    private:
     I2C& bus;
     const I2C::address_t bus_address;
-    bool detected_;
+    bool detected_ = false;
 
     bool readRegister(uint8_t reg, uint16_t& value);
     bool readMultipleRegister(uint8_t reg, uint8_t* data, uint8_t length, bool endTransmission);

firmware/common/message.hpp

@@ -1413,14 +1413,17 @@ class PocsagTosendMessage : public Message {
 class BatteryStateMessage : public Message {
    public:
     constexpr BatteryStateMessage(
+        uint8_t valid_mask,
         uint8_t percent,
         bool on_charger,
         uint16_t voltage)
         : Message{ID::BatteryStateData},
+          valid_mask{valid_mask},
           percent{percent},
           on_charger{on_charger},
           voltage{voltage} {
     }
+    uint8_t valid_mask = 0;
     uint8_t percent = 0;
     bool on_charger = false;
     uint16_t voltage = 0;  // mV

firmware/common/ui_widget.cpp

@@ -2048,7 +2048,7 @@ bool TextField::on_touch(TouchEvent event) {
 
 BatteryIcon::BatteryIcon(Rect parent_rect, uint8_t percent)
     : Widget(parent_rect) {
-    this->set_battery(percent, false);
+    this->set_battery(percent <= 100 ? 1 : 0, percent, false);
     set_focusable(true);
 }
 
@@ -2059,10 +2059,12 @@ void BatteryIcon::getWidgetName(std::string& result) {
     result = "Battery percent";
 }
 
-void BatteryIcon::set_battery(uint8_t percentage, bool charge) {
+void BatteryIcon::set_battery(uint8_t valid_mask, uint8_t percentage, bool charge) {
-    if (charge == charge_ && percent_ == percentage) return;
+    if (charge == charge_ && percent_ == percentage && valid_ == valid_mask) return;
     percent_ = percentage;
     charge_ = charge;
+    valid_ = valid_mask;
+    if ((valid_mask & battery::BatteryManagement::BATT_VALID_VOLTAGE) != battery::BatteryManagement::BATT_VALID_VOLTAGE) percent_ = 102;  // to indicate error
     set_dirty();
 }
 
@@ -2121,7 +2123,7 @@ void BatteryIcon::paint(Painter& painter) {
 
 BatteryTextField::BatteryTextField(Rect parent_rect, uint8_t percent)
     : Widget(parent_rect) {
-    this->set_battery(percent, false);
+    this->set_battery(percent <= 100 ? 1 : 0, percent, false);
     set_focusable(true);
 }
 
@@ -2146,10 +2148,12 @@ void BatteryTextField::getWidgetName(std::string& result) {
     result = "Battery percent";
 }
 
-void BatteryTextField::set_battery(uint8_t percentage, bool charge) {
+void BatteryTextField::set_battery(uint8_t valid_mask, uint8_t percentage, bool charge) {
-    if (charge == charge_ && percent_ == percentage) return;
+    if (charge == charge_ && percent_ == percentage && valid_ == valid_mask) return;
     charge_ = charge;
     percent_ = percentage;
+    valid_ = valid_mask;
+    if ((valid_mask & battery::BatteryManagement::BATT_VALID_VOLTAGE) != battery::BatteryManagement::BATT_VALID_VOLTAGE) percent_ = 102;  // to indicate error
     set_dirty();
 }
 

firmware/common/ui_widget.hpp

@@ -790,7 +790,7 @@ class BatteryTextField : public Widget {
     BatteryTextField(Rect parent_rect, uint8_t percent);
     void paint(Painter& painter) override;
 
-    void set_battery(uint8_t percentage, bool charge);
+    void set_battery(uint8_t valid_mask, uint8_t percentage, bool charge);
     void set_text(std::string_view value);
 
     bool on_key(KeyEvent key) override;
@@ -801,6 +801,7 @@ class BatteryTextField : public Widget {
 
    private:
     uint8_t percent_{102};
+    uint8_t valid_{0};
     bool charge_{false};
 
     Style style{
@@ -816,7 +817,7 @@ class BatteryIcon : public Widget {
 
     BatteryIcon(Rect parent_rect, uint8_t percent);
     void paint(Painter& painter) override;
-    void set_battery(uint8_t percentage, bool charge);
+    void set_battery(uint8_t valid_mask, uint8_t percentage, bool charge);
 
     bool on_key(KeyEvent key) override;
     bool on_touch(TouchEvent event) override;
@@ -826,6 +827,7 @@ class BatteryIcon : public Widget {
 
    private:
     uint8_t percent_{102};
+    uint8_t valid_{0};
     bool charge_{false};
 };