[GitHub] [mynewt-core] kasjer commented on a change in pull request #2382: allow more then one battery

[GitBox <gi...@apache.org>]

kasjer commented on a change in pull request #2382:
URL: https://github.com/apache/mynewt-core/pull/2382#discussion_r497800073



##########
File path: hw/battery/src/battery.c
##########
@@ -25,7 +25,12 @@
 #include <battery/battery_prop.h>
 #include <battery/battery_drv.h>
 
+
+#if MYNEWT_VAL(BATTERY_MAX_COUNT)

Review comment:
       The way to go in that case is to create syscfg definition in **hw/battery/syscfg.yml**.
   Without it package uses value that is never defined.
   something like that:
   ```yml
       BATTERY_MAX_COUNT:
           description: Maximum number of batteries.
           value: 1
   ```
   Then it would be also possible to omit
   ```c
   #if MYNEWT_VAL(BATTERY_MAX_COUNT)
   ```
   ... and just leave it like this:
   ```c
   #define BATTERY_MAX_COUNT MYNEWT_VAL(BATTERY_MAX_COUNT)
   ```

##########
File path: hw/battery/src/battery.c
##########
@@ -818,7 +823,7 @@ battery_init(struct os_dev *dev, void *arg)
     }
     os_mutex_release(&battery_manager.bm_lock);
 
-    assert(i < BATTERY_MAX_COUNT);
+    assert(i <= BATTERY_MAX_COUNT);

Review comment:
       This change is not needed. Condition was correct.
   On the other hand mutex release should be done later and that could be fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org