edouard-mangel
commented
7 years ago You're right, I corrected the mistakes of reimplementing fields already present in the base class, and I'll push it tomorrow after more thorough testing.
Open cguebert opened 7 years ago
edouard-mangel
commented
7 years ago You're right, I corrected the mistakes of reimplementing fields already present in the base class, and I'll push it tomorrow after more thorough testing.
There are a few errors or inconsistencies in the newly added classes. I should have taken the time to take a look at the Pull request and make comments there.
Many fields are redefined in the new classes but are already added by the class they inherit (Request / Response). For example, NCreateRequest::message_id, NCreateResponse::message_id_being_responded_to, NSetRequest::message_id and command_field, NSetResponse::message_id_being_responded_to and status.
NSetRequest redefines command_field as registry::CommandDataSetType!
NSetRequest should not manipulate directly registry::CommandDataSetType, but use the methods of the Message class (has_data_set, get_data_set, set_data_set).
The following code in NSetRequest is not clear for me:
command_data_set_type != DataSetType::PRESENT && command_data_set_type != 0x0101.0x0101if equivalent toDataSetType::ABSENT, so we test here if the data set it neither present nor absent ? The previous point should help correct this code.