Generating a bytearray by choosing an exclusive set of parameters
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
5
down vote
favorite
I am new to python so tips to any part of the code are welcome.
My main problem is how to "overload" the function properly. Is this how supposed to be done with parameters that gets passed exculsivly?
It is intended that only Bytes, Words or a DWord will get passed in the corresponding parameter. (If multiple are passed the longest one is prefered. Sidenote: The naming is based on an old 16bit processor thats why word is 16 bit and Dword is 32)
I also wanted to make the range check of the optional arguments in the beginning but as I default them to None, I had to check for None in the first place before I can check the range and this would make the None check redundant.
I had something like max(Word1,Word2) >= 2**16
in mind, because I hoped in python the max would yield None and the larger would give false, but > is not implemented for None. So to get it to work I used the check for None first. But this seems not elegant.
Afterwards I assign the bigger datachunks to Bytes and raise Errors if the data didn't fit in the range of the type. If nothing is passed the Data is set to 0 to fill up the array in the end.
If no length was given the length calculated by the checks is used.
In the end all the data gets packed into a bytearray (the Message to be sent later). Because bytearray() checks if the elements are in range, the Bytes are not checked seperatly.
Is there a more pythonic way to do it?
def packMessage(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Bytes = [Byte0, Byte1, Byte2, Byte3]
length = 0
# check if the values to be used are in range
if Offset >= 2**16:
raise ValueError
else:
Offset_low = Offset & 0x00FF
Offset_high = (Offset & 0xFF00) >> 8
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError
# split bigger data to bytes and check the data length
if DWord0 is not None:
if 0 <= DWord0 <= 2**32:
Bytes[0] = DWord0 & 0xFF
Bytes[1] = (DWord0 & 0xFF00) >> 8
Bytes[2] = (DWord0 & 0xFF0000) >> 16
Bytes[3] = (DWord0 & 0xFF000000) >> 24
length = 4
else:
raise ValueError
elif Word0 is not None:
if 0 <= Word0 <= 2**16:
Bytes[0] = Word0 & 0xFF
Bytes[1] = (Word0 & 0xFF00) >> 8
else:
raise ValueError
if Word1 is not None:
if 0 <= Word1 <= 2**16:
Bytes[2] = Word1 & 0xFF
Bytes[3] = (Word1 & 0xFF00) >> 8
length = 4
else:
raise ValueError
else:
Word1 = 0
Bytes[2] = Bytes[3] = 0
length = 2
else:
for i in range(len(Bytes)):
if Bytes[i] is not None:
length = i
else:
Bytes[i] = 0
# if no length given take calculated length
if dataLength is None:
dataLength = length
# return the Message
return bytearray([DeviceID, dataLength, Offset_low,
Offset_high, Bytes[0], Bytes[1], Bytes[2], Bytes[3]])
Side note: I decided that the compare order of the Bytes doesn't matter that much so I ditched it for now and replaced it with a loop for now.
It looked like this before:
if Byte0 is not None:
if Byte1 is not None:
if Byte2 is not None:
if Byte3 is not None:
length = 4
else:
Byte3 = 0
else:
length = 3
Byte2 = 0
Byte3 = 0
else:
length = 2
Byte1 = 0
Byte2 = 0
Byte3 = 0
else:
length = 1
Byte0 = 0
Byte1 = 0
Byte2 = 0
Byte3 = 0
I don't see a nice way to wrap this up. So If there is a beautiful solution for this let me know, otherwise feel free to ignore it.
python python-3.x
add a comment |Â
up vote
5
down vote
favorite
I am new to python so tips to any part of the code are welcome.
My main problem is how to "overload" the function properly. Is this how supposed to be done with parameters that gets passed exculsivly?
It is intended that only Bytes, Words or a DWord will get passed in the corresponding parameter. (If multiple are passed the longest one is prefered. Sidenote: The naming is based on an old 16bit processor thats why word is 16 bit and Dword is 32)
I also wanted to make the range check of the optional arguments in the beginning but as I default them to None, I had to check for None in the first place before I can check the range and this would make the None check redundant.
I had something like max(Word1,Word2) >= 2**16
in mind, because I hoped in python the max would yield None and the larger would give false, but > is not implemented for None. So to get it to work I used the check for None first. But this seems not elegant.
Afterwards I assign the bigger datachunks to Bytes and raise Errors if the data didn't fit in the range of the type. If nothing is passed the Data is set to 0 to fill up the array in the end.
If no length was given the length calculated by the checks is used.
In the end all the data gets packed into a bytearray (the Message to be sent later). Because bytearray() checks if the elements are in range, the Bytes are not checked seperatly.
Is there a more pythonic way to do it?
def packMessage(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Bytes = [Byte0, Byte1, Byte2, Byte3]
length = 0
# check if the values to be used are in range
if Offset >= 2**16:
raise ValueError
else:
Offset_low = Offset & 0x00FF
Offset_high = (Offset & 0xFF00) >> 8
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError
# split bigger data to bytes and check the data length
if DWord0 is not None:
if 0 <= DWord0 <= 2**32:
Bytes[0] = DWord0 & 0xFF
Bytes[1] = (DWord0 & 0xFF00) >> 8
Bytes[2] = (DWord0 & 0xFF0000) >> 16
Bytes[3] = (DWord0 & 0xFF000000) >> 24
length = 4
else:
raise ValueError
elif Word0 is not None:
if 0 <= Word0 <= 2**16:
Bytes[0] = Word0 & 0xFF
Bytes[1] = (Word0 & 0xFF00) >> 8
else:
raise ValueError
if Word1 is not None:
if 0 <= Word1 <= 2**16:
Bytes[2] = Word1 & 0xFF
Bytes[3] = (Word1 & 0xFF00) >> 8
length = 4
else:
raise ValueError
else:
Word1 = 0
Bytes[2] = Bytes[3] = 0
length = 2
else:
for i in range(len(Bytes)):
if Bytes[i] is not None:
length = i
else:
Bytes[i] = 0
# if no length given take calculated length
if dataLength is None:
dataLength = length
# return the Message
return bytearray([DeviceID, dataLength, Offset_low,
Offset_high, Bytes[0], Bytes[1], Bytes[2], Bytes[3]])
Side note: I decided that the compare order of the Bytes doesn't matter that much so I ditched it for now and replaced it with a loop for now.
It looked like this before:
if Byte0 is not None:
if Byte1 is not None:
if Byte2 is not None:
if Byte3 is not None:
length = 4
else:
Byte3 = 0
else:
length = 3
Byte2 = 0
Byte3 = 0
else:
length = 2
Byte1 = 0
Byte2 = 0
Byte3 = 0
else:
length = 1
Byte0 = 0
Byte1 = 0
Byte2 = 0
Byte3 = 0
I don't see a nice way to wrap this up. So If there is a beautiful solution for this let me know, otherwise feel free to ignore it.
python python-3.x
@MathiasEttinger I did some rework of the code and also expanded the description. I would appreciate a quick feedback if my post now meets the requirements.
â Kami Kaze
Mar 20 at 8:10
1
Sounds about right, I hope you get some good answers.
â Mathias Ettinger
Mar 20 at 8:56
add a comment |Â
up vote
5
down vote
favorite
up vote
5
down vote
favorite
I am new to python so tips to any part of the code are welcome.
My main problem is how to "overload" the function properly. Is this how supposed to be done with parameters that gets passed exculsivly?
It is intended that only Bytes, Words or a DWord will get passed in the corresponding parameter. (If multiple are passed the longest one is prefered. Sidenote: The naming is based on an old 16bit processor thats why word is 16 bit and Dword is 32)
I also wanted to make the range check of the optional arguments in the beginning but as I default them to None, I had to check for None in the first place before I can check the range and this would make the None check redundant.
I had something like max(Word1,Word2) >= 2**16
in mind, because I hoped in python the max would yield None and the larger would give false, but > is not implemented for None. So to get it to work I used the check for None first. But this seems not elegant.
Afterwards I assign the bigger datachunks to Bytes and raise Errors if the data didn't fit in the range of the type. If nothing is passed the Data is set to 0 to fill up the array in the end.
If no length was given the length calculated by the checks is used.
In the end all the data gets packed into a bytearray (the Message to be sent later). Because bytearray() checks if the elements are in range, the Bytes are not checked seperatly.
Is there a more pythonic way to do it?
def packMessage(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Bytes = [Byte0, Byte1, Byte2, Byte3]
length = 0
# check if the values to be used are in range
if Offset >= 2**16:
raise ValueError
else:
Offset_low = Offset & 0x00FF
Offset_high = (Offset & 0xFF00) >> 8
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError
# split bigger data to bytes and check the data length
if DWord0 is not None:
if 0 <= DWord0 <= 2**32:
Bytes[0] = DWord0 & 0xFF
Bytes[1] = (DWord0 & 0xFF00) >> 8
Bytes[2] = (DWord0 & 0xFF0000) >> 16
Bytes[3] = (DWord0 & 0xFF000000) >> 24
length = 4
else:
raise ValueError
elif Word0 is not None:
if 0 <= Word0 <= 2**16:
Bytes[0] = Word0 & 0xFF
Bytes[1] = (Word0 & 0xFF00) >> 8
else:
raise ValueError
if Word1 is not None:
if 0 <= Word1 <= 2**16:
Bytes[2] = Word1 & 0xFF
Bytes[3] = (Word1 & 0xFF00) >> 8
length = 4
else:
raise ValueError
else:
Word1 = 0
Bytes[2] = Bytes[3] = 0
length = 2
else:
for i in range(len(Bytes)):
if Bytes[i] is not None:
length = i
else:
Bytes[i] = 0
# if no length given take calculated length
if dataLength is None:
dataLength = length
# return the Message
return bytearray([DeviceID, dataLength, Offset_low,
Offset_high, Bytes[0], Bytes[1], Bytes[2], Bytes[3]])
Side note: I decided that the compare order of the Bytes doesn't matter that much so I ditched it for now and replaced it with a loop for now.
It looked like this before:
if Byte0 is not None:
if Byte1 is not None:
if Byte2 is not None:
if Byte3 is not None:
length = 4
else:
Byte3 = 0
else:
length = 3
Byte2 = 0
Byte3 = 0
else:
length = 2
Byte1 = 0
Byte2 = 0
Byte3 = 0
else:
length = 1
Byte0 = 0
Byte1 = 0
Byte2 = 0
Byte3 = 0
I don't see a nice way to wrap this up. So If there is a beautiful solution for this let me know, otherwise feel free to ignore it.
python python-3.x
I am new to python so tips to any part of the code are welcome.
My main problem is how to "overload" the function properly. Is this how supposed to be done with parameters that gets passed exculsivly?
It is intended that only Bytes, Words or a DWord will get passed in the corresponding parameter. (If multiple are passed the longest one is prefered. Sidenote: The naming is based on an old 16bit processor thats why word is 16 bit and Dword is 32)
I also wanted to make the range check of the optional arguments in the beginning but as I default them to None, I had to check for None in the first place before I can check the range and this would make the None check redundant.
I had something like max(Word1,Word2) >= 2**16
in mind, because I hoped in python the max would yield None and the larger would give false, but > is not implemented for None. So to get it to work I used the check for None first. But this seems not elegant.
Afterwards I assign the bigger datachunks to Bytes and raise Errors if the data didn't fit in the range of the type. If nothing is passed the Data is set to 0 to fill up the array in the end.
If no length was given the length calculated by the checks is used.
In the end all the data gets packed into a bytearray (the Message to be sent later). Because bytearray() checks if the elements are in range, the Bytes are not checked seperatly.
Is there a more pythonic way to do it?
def packMessage(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Bytes = [Byte0, Byte1, Byte2, Byte3]
length = 0
# check if the values to be used are in range
if Offset >= 2**16:
raise ValueError
else:
Offset_low = Offset & 0x00FF
Offset_high = (Offset & 0xFF00) >> 8
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError
# split bigger data to bytes and check the data length
if DWord0 is not None:
if 0 <= DWord0 <= 2**32:
Bytes[0] = DWord0 & 0xFF
Bytes[1] = (DWord0 & 0xFF00) >> 8
Bytes[2] = (DWord0 & 0xFF0000) >> 16
Bytes[3] = (DWord0 & 0xFF000000) >> 24
length = 4
else:
raise ValueError
elif Word0 is not None:
if 0 <= Word0 <= 2**16:
Bytes[0] = Word0 & 0xFF
Bytes[1] = (Word0 & 0xFF00) >> 8
else:
raise ValueError
if Word1 is not None:
if 0 <= Word1 <= 2**16:
Bytes[2] = Word1 & 0xFF
Bytes[3] = (Word1 & 0xFF00) >> 8
length = 4
else:
raise ValueError
else:
Word1 = 0
Bytes[2] = Bytes[3] = 0
length = 2
else:
for i in range(len(Bytes)):
if Bytes[i] is not None:
length = i
else:
Bytes[i] = 0
# if no length given take calculated length
if dataLength is None:
dataLength = length
# return the Message
return bytearray([DeviceID, dataLength, Offset_low,
Offset_high, Bytes[0], Bytes[1], Bytes[2], Bytes[3]])
Side note: I decided that the compare order of the Bytes doesn't matter that much so I ditched it for now and replaced it with a loop for now.
It looked like this before:
if Byte0 is not None:
if Byte1 is not None:
if Byte2 is not None:
if Byte3 is not None:
length = 4
else:
Byte3 = 0
else:
length = 3
Byte2 = 0
Byte3 = 0
else:
length = 2
Byte1 = 0
Byte2 = 0
Byte3 = 0
else:
length = 1
Byte0 = 0
Byte1 = 0
Byte2 = 0
Byte3 = 0
I don't see a nice way to wrap this up. So If there is a beautiful solution for this let me know, otherwise feel free to ignore it.
python python-3.x
edited Apr 20 at 9:43
asked Mar 19 at 16:35
Kami Kaze
1305
1305
@MathiasEttinger I did some rework of the code and also expanded the description. I would appreciate a quick feedback if my post now meets the requirements.
â Kami Kaze
Mar 20 at 8:10
1
Sounds about right, I hope you get some good answers.
â Mathias Ettinger
Mar 20 at 8:56
add a comment |Â
@MathiasEttinger I did some rework of the code and also expanded the description. I would appreciate a quick feedback if my post now meets the requirements.
â Kami Kaze
Mar 20 at 8:10
1
Sounds about right, I hope you get some good answers.
â Mathias Ettinger
Mar 20 at 8:56
@MathiasEttinger I did some rework of the code and also expanded the description. I would appreciate a quick feedback if my post now meets the requirements.
â Kami Kaze
Mar 20 at 8:10
@MathiasEttinger I did some rework of the code and also expanded the description. I would appreciate a quick feedback if my post now meets the requirements.
â Kami Kaze
Mar 20 at 8:10
1
1
Sounds about right, I hope you get some good answers.
â Mathias Ettinger
Mar 20 at 8:56
Sounds about right, I hope you get some good answers.
â Mathias Ettinger
Mar 20 at 8:56
add a comment |Â
1 Answer
1
active
oldest
votes
up vote
1
down vote
accepted
Simplest would be to separate parsing of the bytes, words and dwords to their own respective method, this would simplify the main logic a lot
def pack_message2(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
result = parse_dword(DWord0)
if result == ():
# print('no dwords')
result = tuple(parse_words(Word0, Word1))
if result == ():
# print('no words')
result = parse_bytes(Byte0, Byte1, Byte2, Byte3)
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
Even if you wouldn't know what the implementation of each of these methods were, the code would be clear in what happens
offset
parsing the offset is pretty straightforward
def parse_offset(Offset):
if Offset >= 2**16:
raise ValueError('Offset should be less than 2**16')
return (Offset & 0x00FF), ((Offset & 0xFF00) >> 8)
Data
Instead of manually chaining the splitting the words in bytes, you can use that generator expression. This will also allow you to easily parse triple words or even quads without letting the code explode
def parse_dword(dword):
# print('dword:', dword)
if dword is None:
return ()
if not 0 <= dword <= 2**32:
raise ValueError('dword should be between 0 and 2**32')
return tuple((dword & 0xFF * (2**(8*i))) >> (8 * i) for i in range(4))
def parse_words(*words):
# print('words: ', words)
for word in words:
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2**16:
raise ValueError('word should be between 0 and 2**32')
yield from ((word & 0xFF * 2 ** (8*i)) >> (8 * i) for i in range(2))
from itertools import takewhile
def parse_bytes(*bytes_):
# print('bytes: ', bytes_)
return tuple(takewhile(lambda x: x is not None, bytes_))
These methods can be further generalized, but I'll leave that open as an exercise
padding the result
the simplest (but not purest) way to pad a tuple
def pad_tuple_to(tup, to, default=None):
return tup + tuple(default for _ in range(to - len(tup)))
generalization
I was intrigued by the generalization, so I tried it myself
def generalized_parse_word(size, words):
# print('words: ', words)
if words is None:
return 'empty'
for word in words:
# print('word: ', word)
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2 ** (8 * size):
raise ValueError('word with %i should be between 0 and %i' % (size, 2 **(8*size)))
yield from ((word & 0xFF << (8*i)) >> (8 * i) for i in range(size))
def pack_message3(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
data = (
(4, (DWord0,)),
(2, (Word0, Word1)),
(1, (Byte0, Byte1, Byte2, Byte3)),
)
for size, words in data:
result = tuple(generalized_parse_word(size, words))
if result != ():
break
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
arguments
Instead of manually naming the different bytes or words, I would accept a collection as argument, something like this:
def pack_message4(DeviceID, Offset, dataLength=None,
dwords=None, words=None, bytes_=None):
...
data = ((4, dwords), (2, words), (1, bytes))
...
testing
inputs = [
'Word0': 0x1122, 'Word1': 0x3344,
'DWord0': 0x11223344,
'Byte0':34, 'Byte1':17, 'Byte2':68, 'Byte3': 51,
]
for i in inputs:
ans0 = packMessage(DeviceID=3, Offset=220, **i)
ans1 = pack_message3(DeviceID=3, Offset=220, **i)
print(ans0 == ans1, ans0, ans1)
True bytearray(b'x03x04xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
True bytearray(b'x03x04xdcx00D3"x11') bytearray(b'x03x04xdcx00D3"x11')
False bytearray(b'x03x03xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
Could it be your method to calculate the dataLength
in the case of Bytes is off by 1
Thank you for the review. As I am working on another project with a more pressing deadline at the moment, I was not able to read through your suggestions, yet. I will do so as soon as I am returning to the project and will then give my vote and if applicable accept the answer.
â Kami Kaze
Mar 27 at 14:16
Okay putting bits of code in a function was the more or less next logical step. I have problems with the vast amount of data types and their appropiate uses, so this might help me get a better feeling. The optimizations possible by just using another type kind of overwhelm me.
â Kami Kaze
Apr 12 at 14:24
*result
is unpacking the tuple to a list?
â Kami Kaze
Apr 12 at 14:56
you can try it with something like this ´a = (3, 4); print([1, 2, *a, 5])´
â Maarten Fabré
Apr 12 at 19:43
what is the default parameter in the pad_tuple_to for?
â Kami Kaze
Apr 19 at 11:38
 |Â
show 6 more comments
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
1
down vote
accepted
Simplest would be to separate parsing of the bytes, words and dwords to their own respective method, this would simplify the main logic a lot
def pack_message2(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
result = parse_dword(DWord0)
if result == ():
# print('no dwords')
result = tuple(parse_words(Word0, Word1))
if result == ():
# print('no words')
result = parse_bytes(Byte0, Byte1, Byte2, Byte3)
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
Even if you wouldn't know what the implementation of each of these methods were, the code would be clear in what happens
offset
parsing the offset is pretty straightforward
def parse_offset(Offset):
if Offset >= 2**16:
raise ValueError('Offset should be less than 2**16')
return (Offset & 0x00FF), ((Offset & 0xFF00) >> 8)
Data
Instead of manually chaining the splitting the words in bytes, you can use that generator expression. This will also allow you to easily parse triple words or even quads without letting the code explode
def parse_dword(dword):
# print('dword:', dword)
if dword is None:
return ()
if not 0 <= dword <= 2**32:
raise ValueError('dword should be between 0 and 2**32')
return tuple((dword & 0xFF * (2**(8*i))) >> (8 * i) for i in range(4))
def parse_words(*words):
# print('words: ', words)
for word in words:
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2**16:
raise ValueError('word should be between 0 and 2**32')
yield from ((word & 0xFF * 2 ** (8*i)) >> (8 * i) for i in range(2))
from itertools import takewhile
def parse_bytes(*bytes_):
# print('bytes: ', bytes_)
return tuple(takewhile(lambda x: x is not None, bytes_))
These methods can be further generalized, but I'll leave that open as an exercise
padding the result
the simplest (but not purest) way to pad a tuple
def pad_tuple_to(tup, to, default=None):
return tup + tuple(default for _ in range(to - len(tup)))
generalization
I was intrigued by the generalization, so I tried it myself
def generalized_parse_word(size, words):
# print('words: ', words)
if words is None:
return 'empty'
for word in words:
# print('word: ', word)
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2 ** (8 * size):
raise ValueError('word with %i should be between 0 and %i' % (size, 2 **(8*size)))
yield from ((word & 0xFF << (8*i)) >> (8 * i) for i in range(size))
def pack_message3(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
data = (
(4, (DWord0,)),
(2, (Word0, Word1)),
(1, (Byte0, Byte1, Byte2, Byte3)),
)
for size, words in data:
result = tuple(generalized_parse_word(size, words))
if result != ():
break
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
arguments
Instead of manually naming the different bytes or words, I would accept a collection as argument, something like this:
def pack_message4(DeviceID, Offset, dataLength=None,
dwords=None, words=None, bytes_=None):
...
data = ((4, dwords), (2, words), (1, bytes))
...
testing
inputs = [
'Word0': 0x1122, 'Word1': 0x3344,
'DWord0': 0x11223344,
'Byte0':34, 'Byte1':17, 'Byte2':68, 'Byte3': 51,
]
for i in inputs:
ans0 = packMessage(DeviceID=3, Offset=220, **i)
ans1 = pack_message3(DeviceID=3, Offset=220, **i)
print(ans0 == ans1, ans0, ans1)
True bytearray(b'x03x04xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
True bytearray(b'x03x04xdcx00D3"x11') bytearray(b'x03x04xdcx00D3"x11')
False bytearray(b'x03x03xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
Could it be your method to calculate the dataLength
in the case of Bytes is off by 1
Thank you for the review. As I am working on another project with a more pressing deadline at the moment, I was not able to read through your suggestions, yet. I will do so as soon as I am returning to the project and will then give my vote and if applicable accept the answer.
â Kami Kaze
Mar 27 at 14:16
Okay putting bits of code in a function was the more or less next logical step. I have problems with the vast amount of data types and their appropiate uses, so this might help me get a better feeling. The optimizations possible by just using another type kind of overwhelm me.
â Kami Kaze
Apr 12 at 14:24
*result
is unpacking the tuple to a list?
â Kami Kaze
Apr 12 at 14:56
you can try it with something like this ´a = (3, 4); print([1, 2, *a, 5])´
â Maarten Fabré
Apr 12 at 19:43
what is the default parameter in the pad_tuple_to for?
â Kami Kaze
Apr 19 at 11:38
 |Â
show 6 more comments
up vote
1
down vote
accepted
Simplest would be to separate parsing of the bytes, words and dwords to their own respective method, this would simplify the main logic a lot
def pack_message2(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
result = parse_dword(DWord0)
if result == ():
# print('no dwords')
result = tuple(parse_words(Word0, Word1))
if result == ():
# print('no words')
result = parse_bytes(Byte0, Byte1, Byte2, Byte3)
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
Even if you wouldn't know what the implementation of each of these methods were, the code would be clear in what happens
offset
parsing the offset is pretty straightforward
def parse_offset(Offset):
if Offset >= 2**16:
raise ValueError('Offset should be less than 2**16')
return (Offset & 0x00FF), ((Offset & 0xFF00) >> 8)
Data
Instead of manually chaining the splitting the words in bytes, you can use that generator expression. This will also allow you to easily parse triple words or even quads without letting the code explode
def parse_dword(dword):
# print('dword:', dword)
if dword is None:
return ()
if not 0 <= dword <= 2**32:
raise ValueError('dword should be between 0 and 2**32')
return tuple((dword & 0xFF * (2**(8*i))) >> (8 * i) for i in range(4))
def parse_words(*words):
# print('words: ', words)
for word in words:
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2**16:
raise ValueError('word should be between 0 and 2**32')
yield from ((word & 0xFF * 2 ** (8*i)) >> (8 * i) for i in range(2))
from itertools import takewhile
def parse_bytes(*bytes_):
# print('bytes: ', bytes_)
return tuple(takewhile(lambda x: x is not None, bytes_))
These methods can be further generalized, but I'll leave that open as an exercise
padding the result
the simplest (but not purest) way to pad a tuple
def pad_tuple_to(tup, to, default=None):
return tup + tuple(default for _ in range(to - len(tup)))
generalization
I was intrigued by the generalization, so I tried it myself
def generalized_parse_word(size, words):
# print('words: ', words)
if words is None:
return 'empty'
for word in words:
# print('word: ', word)
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2 ** (8 * size):
raise ValueError('word with %i should be between 0 and %i' % (size, 2 **(8*size)))
yield from ((word & 0xFF << (8*i)) >> (8 * i) for i in range(size))
def pack_message3(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
data = (
(4, (DWord0,)),
(2, (Word0, Word1)),
(1, (Byte0, Byte1, Byte2, Byte3)),
)
for size, words in data:
result = tuple(generalized_parse_word(size, words))
if result != ():
break
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
arguments
Instead of manually naming the different bytes or words, I would accept a collection as argument, something like this:
def pack_message4(DeviceID, Offset, dataLength=None,
dwords=None, words=None, bytes_=None):
...
data = ((4, dwords), (2, words), (1, bytes))
...
testing
inputs = [
'Word0': 0x1122, 'Word1': 0x3344,
'DWord0': 0x11223344,
'Byte0':34, 'Byte1':17, 'Byte2':68, 'Byte3': 51,
]
for i in inputs:
ans0 = packMessage(DeviceID=3, Offset=220, **i)
ans1 = pack_message3(DeviceID=3, Offset=220, **i)
print(ans0 == ans1, ans0, ans1)
True bytearray(b'x03x04xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
True bytearray(b'x03x04xdcx00D3"x11') bytearray(b'x03x04xdcx00D3"x11')
False bytearray(b'x03x03xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
Could it be your method to calculate the dataLength
in the case of Bytes is off by 1
Thank you for the review. As I am working on another project with a more pressing deadline at the moment, I was not able to read through your suggestions, yet. I will do so as soon as I am returning to the project and will then give my vote and if applicable accept the answer.
â Kami Kaze
Mar 27 at 14:16
Okay putting bits of code in a function was the more or less next logical step. I have problems with the vast amount of data types and their appropiate uses, so this might help me get a better feeling. The optimizations possible by just using another type kind of overwhelm me.
â Kami Kaze
Apr 12 at 14:24
*result
is unpacking the tuple to a list?
â Kami Kaze
Apr 12 at 14:56
you can try it with something like this ´a = (3, 4); print([1, 2, *a, 5])´
â Maarten Fabré
Apr 12 at 19:43
what is the default parameter in the pad_tuple_to for?
â Kami Kaze
Apr 19 at 11:38
 |Â
show 6 more comments
up vote
1
down vote
accepted
up vote
1
down vote
accepted
Simplest would be to separate parsing of the bytes, words and dwords to their own respective method, this would simplify the main logic a lot
def pack_message2(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
result = parse_dword(DWord0)
if result == ():
# print('no dwords')
result = tuple(parse_words(Word0, Word1))
if result == ():
# print('no words')
result = parse_bytes(Byte0, Byte1, Byte2, Byte3)
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
Even if you wouldn't know what the implementation of each of these methods were, the code would be clear in what happens
offset
parsing the offset is pretty straightforward
def parse_offset(Offset):
if Offset >= 2**16:
raise ValueError('Offset should be less than 2**16')
return (Offset & 0x00FF), ((Offset & 0xFF00) >> 8)
Data
Instead of manually chaining the splitting the words in bytes, you can use that generator expression. This will also allow you to easily parse triple words or even quads without letting the code explode
def parse_dword(dword):
# print('dword:', dword)
if dword is None:
return ()
if not 0 <= dword <= 2**32:
raise ValueError('dword should be between 0 and 2**32')
return tuple((dword & 0xFF * (2**(8*i))) >> (8 * i) for i in range(4))
def parse_words(*words):
# print('words: ', words)
for word in words:
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2**16:
raise ValueError('word should be between 0 and 2**32')
yield from ((word & 0xFF * 2 ** (8*i)) >> (8 * i) for i in range(2))
from itertools import takewhile
def parse_bytes(*bytes_):
# print('bytes: ', bytes_)
return tuple(takewhile(lambda x: x is not None, bytes_))
These methods can be further generalized, but I'll leave that open as an exercise
padding the result
the simplest (but not purest) way to pad a tuple
def pad_tuple_to(tup, to, default=None):
return tup + tuple(default for _ in range(to - len(tup)))
generalization
I was intrigued by the generalization, so I tried it myself
def generalized_parse_word(size, words):
# print('words: ', words)
if words is None:
return 'empty'
for word in words:
# print('word: ', word)
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2 ** (8 * size):
raise ValueError('word with %i should be between 0 and %i' % (size, 2 **(8*size)))
yield from ((word & 0xFF << (8*i)) >> (8 * i) for i in range(size))
def pack_message3(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
data = (
(4, (DWord0,)),
(2, (Word0, Word1)),
(1, (Byte0, Byte1, Byte2, Byte3)),
)
for size, words in data:
result = tuple(generalized_parse_word(size, words))
if result != ():
break
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
arguments
Instead of manually naming the different bytes or words, I would accept a collection as argument, something like this:
def pack_message4(DeviceID, Offset, dataLength=None,
dwords=None, words=None, bytes_=None):
...
data = ((4, dwords), (2, words), (1, bytes))
...
testing
inputs = [
'Word0': 0x1122, 'Word1': 0x3344,
'DWord0': 0x11223344,
'Byte0':34, 'Byte1':17, 'Byte2':68, 'Byte3': 51,
]
for i in inputs:
ans0 = packMessage(DeviceID=3, Offset=220, **i)
ans1 = pack_message3(DeviceID=3, Offset=220, **i)
print(ans0 == ans1, ans0, ans1)
True bytearray(b'x03x04xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
True bytearray(b'x03x04xdcx00D3"x11') bytearray(b'x03x04xdcx00D3"x11')
False bytearray(b'x03x03xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
Could it be your method to calculate the dataLength
in the case of Bytes is off by 1
Simplest would be to separate parsing of the bytes, words and dwords to their own respective method, this would simplify the main logic a lot
def pack_message2(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
result = parse_dword(DWord0)
if result == ():
# print('no dwords')
result = tuple(parse_words(Word0, Word1))
if result == ():
# print('no words')
result = parse_bytes(Byte0, Byte1, Byte2, Byte3)
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
Even if you wouldn't know what the implementation of each of these methods were, the code would be clear in what happens
offset
parsing the offset is pretty straightforward
def parse_offset(Offset):
if Offset >= 2**16:
raise ValueError('Offset should be less than 2**16')
return (Offset & 0x00FF), ((Offset & 0xFF00) >> 8)
Data
Instead of manually chaining the splitting the words in bytes, you can use that generator expression. This will also allow you to easily parse triple words or even quads without letting the code explode
def parse_dword(dword):
# print('dword:', dword)
if dword is None:
return ()
if not 0 <= dword <= 2**32:
raise ValueError('dword should be between 0 and 2**32')
return tuple((dword & 0xFF * (2**(8*i))) >> (8 * i) for i in range(4))
def parse_words(*words):
# print('words: ', words)
for word in words:
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2**16:
raise ValueError('word should be between 0 and 2**32')
yield from ((word & 0xFF * 2 ** (8*i)) >> (8 * i) for i in range(2))
from itertools import takewhile
def parse_bytes(*bytes_):
# print('bytes: ', bytes_)
return tuple(takewhile(lambda x: x is not None, bytes_))
These methods can be further generalized, but I'll leave that open as an exercise
padding the result
the simplest (but not purest) way to pad a tuple
def pad_tuple_to(tup, to, default=None):
return tup + tuple(default for _ in range(to - len(tup)))
generalization
I was intrigued by the generalization, so I tried it myself
def generalized_parse_word(size, words):
# print('words: ', words)
if words is None:
return 'empty'
for word in words:
# print('word: ', word)
if word is None:
# yield from (0, 0)
break
if not 0 <= word <= 2 ** (8 * size):
raise ValueError('word with %i should be between 0 and %i' % (size, 2 **(8*size)))
yield from ((word & 0xFF << (8*i)) >> (8 * i) for i in range(size))
def pack_message3(DeviceID, Offset, dataLength=None,
Byte0=None, Byte1=None, Byte2=None, Byte3=None,
Word0=None, Word1=None, DWord0=None):
Offset_low, Offset_high = parse_offset(Offset)
if dataLength is not None and not 1 <= dataLength <= 4:
raise ValueError('dataLength should be between 1 and 4')
data = (
(4, (DWord0,)),
(2, (Word0, Word1)),
(1, (Byte0, Byte1, Byte2, Byte3)),
)
for size, words in data:
result = tuple(generalized_parse_word(size, words))
if result != ():
break
if dataLength is None:
dataLength = len(result)
result = pad_tuple_to(result, 4, 0) # if result should be padded with 0's'
# print(result, dataLength)
return bytearray([DeviceID, dataLength, Offset_low, Offset_high, *result])
arguments
Instead of manually naming the different bytes or words, I would accept a collection as argument, something like this:
def pack_message4(DeviceID, Offset, dataLength=None,
dwords=None, words=None, bytes_=None):
...
data = ((4, dwords), (2, words), (1, bytes))
...
testing
inputs = [
'Word0': 0x1122, 'Word1': 0x3344,
'DWord0': 0x11223344,
'Byte0':34, 'Byte1':17, 'Byte2':68, 'Byte3': 51,
]
for i in inputs:
ans0 = packMessage(DeviceID=3, Offset=220, **i)
ans1 = pack_message3(DeviceID=3, Offset=220, **i)
print(ans0 == ans1, ans0, ans1)
True bytearray(b'x03x04xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
True bytearray(b'x03x04xdcx00D3"x11') bytearray(b'x03x04xdcx00D3"x11')
False bytearray(b'x03x03xdcx00"x11D3') bytearray(b'x03x04xdcx00"x11D3')
Could it be your method to calculate the dataLength
in the case of Bytes is off by 1
edited Apr 20 at 14:14
answered Mar 21 at 16:28
Maarten Fabré
3,204214
3,204214
Thank you for the review. As I am working on another project with a more pressing deadline at the moment, I was not able to read through your suggestions, yet. I will do so as soon as I am returning to the project and will then give my vote and if applicable accept the answer.
â Kami Kaze
Mar 27 at 14:16
Okay putting bits of code in a function was the more or less next logical step. I have problems with the vast amount of data types and their appropiate uses, so this might help me get a better feeling. The optimizations possible by just using another type kind of overwhelm me.
â Kami Kaze
Apr 12 at 14:24
*result
is unpacking the tuple to a list?
â Kami Kaze
Apr 12 at 14:56
you can try it with something like this ´a = (3, 4); print([1, 2, *a, 5])´
â Maarten Fabré
Apr 12 at 19:43
what is the default parameter in the pad_tuple_to for?
â Kami Kaze
Apr 19 at 11:38
 |Â
show 6 more comments
Thank you for the review. As I am working on another project with a more pressing deadline at the moment, I was not able to read through your suggestions, yet. I will do so as soon as I am returning to the project and will then give my vote and if applicable accept the answer.
â Kami Kaze
Mar 27 at 14:16
Okay putting bits of code in a function was the more or less next logical step. I have problems with the vast amount of data types and their appropiate uses, so this might help me get a better feeling. The optimizations possible by just using another type kind of overwhelm me.
â Kami Kaze
Apr 12 at 14:24
*result
is unpacking the tuple to a list?
â Kami Kaze
Apr 12 at 14:56
you can try it with something like this ´a = (3, 4); print([1, 2, *a, 5])´
â Maarten Fabré
Apr 12 at 19:43
what is the default parameter in the pad_tuple_to for?
â Kami Kaze
Apr 19 at 11:38
Thank you for the review. As I am working on another project with a more pressing deadline at the moment, I was not able to read through your suggestions, yet. I will do so as soon as I am returning to the project and will then give my vote and if applicable accept the answer.
â Kami Kaze
Mar 27 at 14:16
Thank you for the review. As I am working on another project with a more pressing deadline at the moment, I was not able to read through your suggestions, yet. I will do so as soon as I am returning to the project and will then give my vote and if applicable accept the answer.
â Kami Kaze
Mar 27 at 14:16
Okay putting bits of code in a function was the more or less next logical step. I have problems with the vast amount of data types and their appropiate uses, so this might help me get a better feeling. The optimizations possible by just using another type kind of overwhelm me.
â Kami Kaze
Apr 12 at 14:24
Okay putting bits of code in a function was the more or less next logical step. I have problems with the vast amount of data types and their appropiate uses, so this might help me get a better feeling. The optimizations possible by just using another type kind of overwhelm me.
â Kami Kaze
Apr 12 at 14:24
*result
is unpacking the tuple to a list?â Kami Kaze
Apr 12 at 14:56
*result
is unpacking the tuple to a list?â Kami Kaze
Apr 12 at 14:56
you can try it with something like this ´a = (3, 4); print([1, 2, *a, 5])´
â Maarten Fabré
Apr 12 at 19:43
you can try it with something like this ´a = (3, 4); print([1, 2, *a, 5])´
â Maarten Fabré
Apr 12 at 19:43
what is the default parameter in the pad_tuple_to for?
â Kami Kaze
Apr 19 at 11:38
what is the default parameter in the pad_tuple_to for?
â Kami Kaze
Apr 19 at 11:38
 |Â
show 6 more comments
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f189948%2fgenerating-a-bytearray-by-choosing-an-exclusive-set-of-parameters%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
@MathiasEttinger I did some rework of the code and also expanded the description. I would appreciate a quick feedback if my post now meets the requirements.
â Kami Kaze
Mar 20 at 8:10
1
Sounds about right, I hope you get some good answers.
â Mathias Ettinger
Mar 20 at 8:56