BattAnalog: Allow users to define the cell count manually #144
Conversation
Even though the current automatic cell count detection logic works fine for the batteries with lower cell counts, it does not work as expected for the batteries with higher cell counts under some circumstances. For example, 45.6V total battery voltage could either mean 4.14V per cell for an 11S battery or 3.8V per cell for a 12S battery. In this case, the logic in use decides that the battery is 11S and gives wrong per cell voltage if the battery in use is 12S. This change allows users to optionally define the cell count manually in the settings of this widget to prevent the type of problems described above.
|
we can not do that, we are limited to 5 option only In addition to the future, if you add an option in the middle, you should take care of backward compatibility to existing instances, since the edgeTx is keeping the user selection by order, and not by key-value pairs ;-( after your changes, since the Lithium_Ion is now in position [5] |
|
Thanks a lot for your reply. Now that you pointed out, I realized that "Lithium_HV" is missing in the options menu after these changes. I guess I focused too much on the functionality I was adding while testing and missed that detail. If it is not possible to combine the existing battery type options in a single option, would it be feasible to use global variables to set the cell count? Similar to what the "Flights" widget does with flight counts. |
|
Easy to do, hard to explain to users. If we could get a simple storage space in the model (like the jeti api have) , i could do mach more |
|
Please close this PR so it will not be merged by mistake |
|
Little chance of that... I was going to ping you to review it since it is your baby anyway, plus I was thinking we were still limited to five options unless I missed a memo or PR... 😱 Back to the drawing board for this I'm afraid.
Hm... do I hear an enhancement issue that hasn't been raised? 🤪 Sounds interesting (and useful)... |
|
It was raised, during the dev session, |
|
At the EdgeTX fest, I was told that the the number of the options we are allowed to have in a widget is increased (to 10 I believe?). I haven't verified it myself, but if that is the case, do you mind if I revive this MR? |
|
There is no need to revive this issue |
|
Cool. Thank you. |
|
We now just need to wait to v2.11 |
Even though the current automatic cell count detection logic works fine for the batteries with lower cell counts, it does not work as expected for the batteries with higher cell counts under some circumstances. For example, 45.6V total battery voltage could either mean 4.14V per cell for an 11S battery or 3.8V per cell for a 12S battery. In this case, the logic in use decides that the battery is 11S and gives wrong per cell voltage if the battery in use is 12S.
This change allows users to optionally define the cell count manually in the settings of this widget to prevent the type of problems described above.