findNotUsedBits() reports the signals index+1, not the index

[altendky]

canmatrix/src/canmatrix/canmatrix.py

Lines 711 to 745 in df1c7b7

	def findNotUsedBits(self):
	"""
	Find unused bits in frame
	:return: dict with position and length-tuples
	"""
	bitfield = []
	bitfieldLe = []
	bitfieldBe = []
	for i in range(0,64):
	bitfieldBe.append(0)
	bitfieldLe.append(0)
	bitfield.append(0)
	i = 0
	for sig in self.signals:
	i += 1
	for bit in range(sig.getStartbit(), sig.getStartbit() + int(sig.size)):
	if sig.is_little_endian:
	bitfieldLe[bit] = i
	else:
	bitfieldBe[bit] = i
	for i in range(0,8):
	for j in range(0,8):
	bitfield[i*8+j] = bitfieldLe[i*8+(7-j)]
	for i in range(0,8):
	for j in range(0,8):
	if bitfield[i*8+j] == 0:
	bitfield[i*8+j] = bitfieldBe[i*8+j]
	return bitfield

1. I would think this should be filling out indexes that can actually be used to index the signal list, or just filling out the actual signal. I tend towards the latter unless there's a specific reason anyone needs to care about the index.
2. Use for index, signal in enumerate(self.signals): if we are going to stick with the indexes.
3. Regardless, None is a much more clear indicator of nothing than 0.
4. In my packing code (being ported in in Fix frame decoding #224) there is allowance for overlapping signals. To retain that each element should be a list, not a single value. If this is used then an empty list would indicate nothing as opposed to None.

[ebroecker]

Firstofall:
I wrote this function once, but never had a real usecase for it.
Originally I wanted to create dummy signals for not used bits in a frame, so that all bits are used.
findNotUsedBits was a helper function for it.
There may be usecases for it, but I do not think anyone is using this method for now.

I agree, None would be the better approach, than 0, thus using the signal-index (and not signal-index+1) the way to do it than.
I also aggree in 4., that a list for each index would be nice (overlapping also occures while using multiplex...)

[altendky]

This also seems like it should be quite related to packing code, just inserting a marker for the signal rather than some bit of the encoded signal. Like maybe the packing code should have an option as to what it inserts to avoid a secondary implementation? Maybe, I dunno. Or just drop it and recreate if needed based on whatever packing code we end up with in the end.