shining-man / bsc_fw

Firmware battery safety controller (BSC)
MIT License
81 stars 15 forks source link

Fix bitfield handling in JkBms #92

Closed meikjaeckle closed 3 months ago

meikjaeckle commented 4 months ago

This pull request fixes the issue as described by https://github.com/shining-man/bsc_fw/issues/70

I found a library type_safe::flag_set, that implements the requirements similar to what I talked about in the issue description. I used that instead of an own implementation.

Note: I changed the library for the bit handling from PortableBitfields to type_safe::flag_set. The PortableBitfields library was a little to wasteful regarding RAM usage. A bit set of 8 single bits required 8bytes to store the value, one byte for each bit and the used base type depends on the number of used bits. So for 16 single bits you already needs 32bytes :-( Therefore the type_safe::flag_set from the type_safe library of Jonathan Mueller just uses the type that is really required and has all in all minimal overhead. Defining an own flag_set is really simple:

  enum class MyFlags
  {
      FLAG_1,
      FLAG_2,
      FLAG_3,
      _flag_set_size,
  };
  using FlagMsg = type_safe::flag_set<MyFlags>;

And using it is simple as well:

const uint8_t rawValue = 0x21;
FlagMsg msg = FlagMsg::from_int(rawValue);
// Check if a single bit is set
if (msg.is_set(MyFlags::FLAG1))
  std::cout << "Bit is set" << std::endl;
// Set a single bit by calling set() method
msg.set(MyFlags::FLAG1);
// Set a single bit by operator|=
msg |= MyFlags::FLAG1;
// and so on...

In addition to the new flag_set implementation, I changed the #defines for the "BMS_ERRSTATUS..." values to constexpr with additional helper methods to convert the bit number into a decimal value.

I added googletest as test framework to the platformio configuration and added the test run for native build in the CI build.

The helper and types for the JkBms are verified by unit tests.

meikjaeckle commented 4 months ago

I renamed my branch, that`s why I had to create a new PR, sorry for that.