Closed dgoo2308 closed 4 years ago
:green_heart:
Calculating CRC is costly; should have it after every other validation that might narrow the execution cost down. That was why we put it down in the execution path after address check and length check. Especially on a busy network, it will push every device to the limit.
Maybe just store the STATUS_ILLEGAL_FUNCTION
in a variable and send it back if the CRC is also correct? This allows the device to skip CRC check for valid or invalid requests not targetted toward it and fixes the issue in question without adding additional performance cost to the function.
@falahati noted, will look into that possibility, I see now the reason behind it, thanks!
as per @IanAber (#61) prevent sending STATUS_ILLEGAL_FUNCTION on a bogus frame:
validateRequest()
: move CRC and validate address to start of procedureSome extras for extra snappy performance optimalisations:
relevantAddress()
: keep the check it local in , since we provide theunitAddress
to check for broadcast.validateRequest()
: keep is_Broadcast check local , it's all here i.s.o. calling isBroadcast() =>calling readUnitAddress()
: inline withcreateResponse()
executeCallback
: remove boolisbroadcast
and avoid thus extra branch check and memory usage