-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add initial E1.33 RDM tests #1953
Add initial E1.33 RDM tests #1953
Conversation
…into e1.33-rdm-tests
@@ -7923,6 +7923,791 @@ class SetInterfaceHardwareAddressType1WithData( | |||
PID = 'INTERFACE_HARDWARE_ADDRESS_TYPE1' | |||
|
|||
|
|||
# E1.33/E1.37-7 PIDS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this auto-generated code? I would have gotten mad writing that by hand ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah list_rdm_tests.py ( https://github.com/OpenLightingProject/ola/blob/af3b2a1e3934d529b1a5b878d7423f990ab81f96/tools/rdm/list_rdm_tests.py ) does as much as it can, which generally covers writing most of the easiest failing tests and the stub of the working ones, you just have to supply the valid data and then validate the results (at the simplest level).
It also (hence the name) lists all test types that should exist (e.g. AllSubDevicesGetFoo) so we can compare that against what we've currently got and work out what's missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An eyeball would still help though, as e.g. I noticed we've no magic casing so the RDM ones were FooRdmBar at first until I manually fixed them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eases my mind that you didn't have to type it all by hand.
I did read it all and didn't find any glaring discrepancies. The only thing I saw multiple times is the naming "withExtraData" vs. "with more than 2 bytes of data"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the class of test named ...WithExtraData, have at least one additional byte on the end of what should ideally otherwise be a valid request, the responder should then NAck them for sending too much data. The "with more than 2 bytes" value is calculated based on how much data they should have, I think most of these will probably be things like endpoints which are two bytes long, hence the repetition.
In comparison say GetDMXPersonalityDescriptionWithExtraData says with more than 1 byte of data.
f87e79d
into
OpenLightingProject:master
After #1952 .