Generating a bytearray by choosing an exclusive set of parameters

The name of the pictureThe name of the pictureThe name of the pictureClash 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.







share|improve this question





















  • @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
















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.







share|improve this question





















  • @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












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.







share|improve this question













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.









share|improve this question












share|improve this question




share|improve this question








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
















  • @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










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






share|improve this answer























  • 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










Your Answer




StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);








 

draft saved


draft discarded


















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






























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






share|improve this answer























  • 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














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






share|improve this answer























  • 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












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






share|improve this answer















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







share|improve this answer















share|improve this answer



share|improve this answer








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
















  • 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












 

draft saved


draft discarded


























 


draft saved


draft discarded














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













































































Popular posts from this blog

Greedy Best First Search implementation in Rust

Function to Return a JSON Like Objects Using VBA Collections and Arrays

C++11 CLH Lock Implementation