Add battery_management_system_protocol

[Jakeler]

No description provided.

[generalmimon]

@Jakeler Can you please share any sample files/captures for testing?

[Jakeler]

@generalmimon
Sure, I have just rewritten my test cases: https://github.com/Jakeler/bms-parser/blob/master/py/test.py
Also some more examples are here: https://github.com/Jakeler/bms-parser/blob/master/py/checksum.py#L19
And a full communication log: https://github.com/Jakeler/bms-parser/blob/master/dumps/pc.log

[Jakeler]

Anything left that blocks merging?

[KOLANICH]

I guess read_req is the same as write_req, just without a payload. I wonder if it can be validated.

[generalmimon]

hardware/battery/battery_management_system_protocol.ksy

Please move the spec one level up to hardware/battery_management_system_protocol.ksy. battery/ is not a great name for a subdirectory - it's too specific. It's quite likely that it would stay alone in this subfolder forever.

Besides, I don't like this third dimension (directory - directory - file) anarchy - in most cases it does not make any sense, like having a directory serialization/asn1/ with a single file asn1_der.ksy.

[KOLANICH]

These were just some notes. Though I don't think that fixing them would accelerate the merging: if you looked at PR counter you will see that there is plenty of non-merged PRs here. Some are ignored for several years. Though having a PR into this repo is still beneficial since it increases visibility and discoverability of your spec (there were several cases that 2 specs for the same format were implemented by 2 people simultaneously because of lack of communication, essentially wasting the effort).

[KOLANICH]

Please move the spec one level up to hardware/battery_management_system_protocol.ksy. battery/ is not a great name for a subdirectory - it's too specific. It's quite likely that it would stay alone in this subfolder forever.

The subdir was created because I have asked that. There are other battery-related formats, like SmartBattery which I have plans to implement in some future (though IDK how far is that future, I'd surely want to play a bit with some laptops batteries, but don't have currently time for that).

does not make any sense, like having a directory serialization/asn1/ with a single file asn1_der.ksy.

IMHO it perfectly makes sense. It allows not to move specs from one dir into another when more ASN.1 specs are added.

[generalmimon]

There are other battery-related formats, like SmartBattery which I have plans to implement in some future (though IDK how far is that future, I'd surely want to play a bit with some laptops batteries, but don't have currently time for that).

I'm sure of that. But I think that for such still relatively low number of format specifications in KSF right now, the greater depth of the directory tree is rather detrimental than beneficial - the added level of subdirectories currently does not help in maintaining the directory structure balanced. It merely opens a room for a total anarchy - there are no established rules what qualifies for being a balanced subdirectory name (or rather a topic). If there is just a flat structure of top-level directories, most of which make some sense (though some less than others, that's true - for example machine_code is pretty useless, why not just hardware?), it's easier for the implementers to just pick the most appropriate directory and put their spec in there without worrying about whether they should introduce a new subdirectory because someday in the future (like 5-10 years from now) might arise 1-2 new specifications that fit into the same subfolder.

I would not try to predict future and solve problems that do not exist - only once we have too many files in the individual top-level categories and it will become apparent that they could be split into several balanced subgroups, it will perhaps make sense.

[generalmimon]

I was wondering what happens if sum(body) > 0x10000 and I was surprised that the format authors would not think about it, but I believe that you did not quite follow the specification (page 2):

Red is the checked byte, the sum of all bytes; the latter two are the checked result, which is the sum of all the previous checks and reverses + 1.

It is a little bit weird English, but it makes sense to me and it also matches your interpretation (but solves the problem in case sum(body) > 0x10000).

For this value of checksum_input (actually taken from https://github.com/Jakeler/bms-parser/blob/dfb9e05e3f4ee0e70cd71ab2114bff1c66661899/py/test.py#L10):

checksum_input_str = '00 1b 11 38 00 62 00 a4 04 b0 00 00 27 6e 02 82 00 00 00 00 21 0e 03 0b 02 0b 22 0b 10'
checksum_input = bytes.fromhex(checksum_input_str)

the sum is

print(sum(checksum_input)) # => 958

The expected_checksum is then

print(0x10000 - 958) # 64578

according to you, and

print((~958 + 1) & 0xffff) # 64578
# which can be using the relationship -a = ~a + 1
# (from definition of the [two's complement representation]
# (https://en.wikipedia.org/wiki/Two%27s_complement)) equivalently rewritten as:
print(-958 & 0xffff) # 64578

according to the spec.

In fact, 0x10000 - sum(checksum_input) would work, but it would also have to be clamped to the lower 16 bits like (0x10000 - sum(checksum_input)) & 0xffff. However, if you only take the lower 16 bits from 0x10000 - a, it's the same as if you did 0x7af20000 - a, 0xff0000 - a or just 0x0000 - a, because all these values have the value of 0 as far as the 16 bits are concerned.

[Jakeler]

I admit that I have simplified the formula a bit, but this is really just a theoretical problem. Even if we are assuming the complete data consists of 0xff... you would need 256 bytes to reach the 16 bits max value. The data length field is just one byte, so the body can not be larger than 256 bytes.
Yes, there is also a status byte (which is 0x00 on every valid packet) and the length itself, but real data is never always 0xff of course, so this easily would work in practice even if the length is pushed to the limit.

In reality there are even earlier limits, for example the balancing flags are for max. 32 cells, that would produce only 64 + 2 = 66 bytes in the cell voltages response and the other structures are constant size.
But I can add a note to the doc...

[generalmimon]

I admit that I have simplified the formula a bit, but this is really just a theoretical problem.

Perhaps, but I don't perceive this as a solid reason to intentionally write a checksum formula there that does not work in general case, especially when the official one in the original specification does. That's not how reliable software is written. What is the point? To simplify the calculation? Is it worth to study the entire format and evaluate all its edge cases to make sure that in absolutely no case cannot the sum of the payload bytes exceed 65536, only to be able to "simplify" the expression from -sum(checksum_input) & 0xffff to 0x10000 - sum(checksum_input)? What if the format will be extended at some point in the future (some structure will be added) and the sum(checksum_input) will be able to go over that 65536?

[Jakeler]

the added level of subdirectories currently does not help in maintaining the directory structure balanced. It merely opens a room for a total anarchy - there are no established rules what qualifies for being a balanced subdirectory name (or rather a topic).

I agree with that, even more directories don't make it better, especially if there are only 1-2 specs in it. I will move this one up.

If there is just a flat structure of top-level directories, most of which make some sense (though some less than others, that's true - for example machine_code is pretty useless, why not just hardware?), it's easier for the implementers to just pick the most appropriate directory and put their spec in there without worrying about whether they should introduce a new subdirectory because someday in the future (like 5-10 years from now) might arise 1-2 new specifications that fit into the same subfolder.

That being said, from my viewpoint (being pretty new in kaitai) the top level structure is in its current state is already incredibly confusing. Looking just at the folders I can't decide where some specs should be. For example what even means hardware, it is definitions for hardware (3d models, schematic formats) or stuff that runs close to hardware or everything somewhat related? First thing that comes to my mind for "hardware and digital formats" is firmware, but this is here in another top level folder... Looking at the current hardware contents it is more like communication protocols between hardware.
It is very inconsistent, most of them are general topics (where it is used) but then there are folder for specific products like windows and macos, suddenly grouping by a different metric (where it comes from) and creating conflicts. For example if I want the windows event log format I would look in windows, but it is in log instead... What is higher priority here?
At the same time there is no linux/unix folder and typical formats like systemd-journal or sudo are scattered over the general topics, again being inconsistent.Why is there no os (operating systems) for all of them?
Then there are also too generic folders like security - what should be in there? Many things are somewhat related to security, but it is again a different metric and causing conflicts with grouping by usage. For example I would expect efivar signatures in firmware not security... maybe call it cryptography for key formats etc.
IMHO the whole tree should be reorganized into a low number of general folders, grouped by usage of the formats. Some depth is not always bad (2-3 levels max). For example hardware/firmware, media/image, media/font, media/game, (media/audio?), os/filesystem, os/executable, os/log would be sensible subdirectories...

Also I am sure you deeply involved guys have an idea in your head about what these categories should contain, but especially for newcomers it would be helpful to have a short definition of every category/folder in the readme as well.

[Jakeler]

These were just some notes. Though I don't think that fixing them would accelerate the merging: if you looked at PR counter you will see that there is plenty of non-merged PRs here. Some are ignored for several years. Though having a PR into this repo is still beneficial since it increases visibility and discoverability of your spec (there were several cases that 2 specs for the same format were implemented by 2 people simultaneously because of lack of communication, essentially wasting the effort).

Yeah, I already saw that, but why? It would increase the visibility even more if stuff gets actually merged at some point...

[KOLANICH]

It is very inconsistent, most of them are general topics (where it is used) but then there are folder for specific products like windows and macos, suddenly grouping by a different metric (where it comes from) and creating conflicts. For example if I want the windows event log format I would look in windows, but it is in log instead... What is higher priority here?

I absolutely agree that the current system is complete mess. I also would like to remove the dirs named after OSes, create 0d, 1d, 2d/{vector,raster}, 3d/{mesh,voxel,structural} in the root, move image into 2d into the needed dir, so do with CAD for drawings. Then move 3d models from media into 3d.

[KOLANICH]

Here is the spec for dirs metadata. kaitai-io/kaitai_struct_doc#48 . Now we should decide where and how (create a PR into this repo, create a separate temporary repo from which the file wil be copied into this repo discarding all the history of its editing, or maybe a wiki page with a yaml block) are we going to cooperate on it.

[generalmimon]

You don't need to write it here, it is sufficiently delimited by the checksum_input attribute. Anyone can simply access checksum_input and they will directly obtain the byte array that's needed to calculate the checksum. It is also apparent from the spec where these bytes come from.