Simplify code flow for broadcast requests #2720
Conversation
janiversen
left a comment
janiversen
left a comment
There was a problem hiding this comment.
I like simplification, but we must ensure never to send a response if we are in broadcast mode.
Seems there was also some CI problems.
| self.server_send(response, self.last_addr) | ||
| response.transaction_id = self.last_pdu.transaction_id | ||
| response.dev_id = self.last_pdu.dev_id | ||
| self.server_send(response, self.last_addr) |
There was a problem hiding this comment.
hmmm seems to me this change will send a response if we are in broadcast mode, that is not allowed.
There was a problem hiding this comment.
No, there is an early return in broadcast mode and this code will never execute.
| context = self.server.context[self.last_pdu.dev_id] | ||
| response = await self.last_pdu.update_datastore(context) | ||
| await self.last_pdu.update_datastore(self.server.context[dev_id]) | ||
| return |
There was a problem hiding this comment.
broadcast mode returns here without responses
janiversen
left a comment
janiversen
left a comment
There was a problem hiding this comment.
What was wrong with the old way ??? it seems more efficient to use asyncio.gather
I'm not sure, but it was causing the test failure so there must be some race condition? (My local I can investigate in a separate PR. |
janiversen
left a comment
janiversen
left a comment
There was a problem hiding this comment.
That is an explanation I can understand, and performance at this point is not very important (nor is your solution very slow).
A small simplification - return early in the broadcast case and don't bother with
response.(
pyrighthad a false positive here, but it leads to simpler code)