Fix USB PRO change-of-state packet handling to allow valid packets, closes #1290 - #1301
Conversation
peternewman
left a comment
peternewman
left a comment
There was a problem hiding this comment.
Just a few minor changes please.
|
|
||
| if (length < sizeof(widget_data_changed)) { | ||
| OLA_WARN << "Change of state packet was too small: " << length; | ||
| if ((length < 7) || (length > sizeof(widget_data_changed))) { |
There was a problem hiding this comment.
Can we have a comment in the code where the 7 came from please.
| if (length < sizeof(widget_data_changed)) { | ||
| OLA_WARN << "Change of state packet was too small: " << length; | ||
| if ((length < 7) || (length > sizeof(widget_data_changed))) { | ||
| OLA_WARN << "Change of state packet was invalid: " << length; |
There was a problem hiding this comment.
Can we change this to "was an invalid size" or similar, we've not actually looked at if the data is valid yet.
|
|
||
| if (length < sizeof(widget_data_changed)) { | ||
| OLA_WARN << "Change of state packet was too small: " << length; | ||
| if ((length < 7) || (length > sizeof(widget_data_changed))) { |
There was a problem hiding this comment.
Likewise for the second implementation.
peternewman
changed the title
|
@nomis52 it seems rather odd we need to duplicate this code for the Enttec Pro and the Generic USBPro, although I can't see any easy way to resolve it as the inheritance diagrams don't seem to overlap at all. Probably one for another day I guess. |
|
I'm not too famliar with how this works, but pushing to my branch on github caused immediate changes here in this window, so I think the pull is now including the newly added changes as well.... My own compile-test now worked. Thanks for the help. P.S. apparently some unit tests are failing (in the .3 and .4 tasks), causing the travis-ci test to fail. Anything I can do? |
|
Yes, because it's a PR, pushing to your branch does the trick. Some of the unit tests are a tad flaky so I'll rerun them when the others have finished. |
3 participants
seems to have worked... :-)