Add cmd error code descr #155
Conversation
canteuni
force-pushed
the
add_cmd_error_code_descr
branch
from
ae3ffc1
to
0d4b8f1
Compare
|
Hi, very good idea. But I think it is not thought far enough. We have to take into account that there are netfn and group ids which have commands with the same id and have to be distinguished between them. Have a look here: https://github.com/kontron/python-ipmi/blob/master/pyipmi/msgs/registry.py#L48C8-L48C73 what do you think |
|
Oh you're right, I thought that the command id was globally unique but that's not the case. Then I must use a tuple |
canteuni
force-pushed
the
add_cmd_error_code_descr
branch
from
0d4b8f1
to
b4163c2
Compare
pyipmi/chassis.py
Outdated
| check_completion_code(rsp.completion_code) | ||
| check_completion_code( | ||
| rsp.completion_code, | ||
| req.cmdid, |
There was a problem hiding this comment.
Is there a reason you do use the req here? Above you use rsp.
There was a problem hiding this comment.
On the other hand, would'nt it be better to have a new function like check_rsp_completion_code(rsp)?
There was a problem hiding this comment.
Wow that's my bad again, didn't sleep enough I guess. Sorry for this.
Yes I was asking myself the same thing but wasn't sure, if you think a function check_rsp_completion_code(rsp) comes handy too I will add it !
There was a problem hiding this comment.
Adding a new function it will reduce the linses when checking the cc and you only have to pass the rsp parameter.
There was a problem hiding this comment.
Apart from that it looks good!
canteuni
force-pushed
the
add_cmd_error_code_descr
branch
2 times, most recently
from
d45b91c
to
b9d4fe8
Compare
pyipmi/msgs/constants.py
Outdated
| @@ -383,6 +385,48 @@ | |||
| 0x86: 'requested maximum privilege level exceeds user and/or channel' | |||
| ' privilege limit', | |||
| }, | |||
| (NETFN_APP | 1, CMDID_SET_SESSION_PRIVILEGE_LEVEL, None): { | |||
There was a problem hiding this comment.
I choose to set NefFn | 1 as the key in the description dictionary rather than just NetFn because we use the dictionary only for IPMI Responses, but the choice can be debated.
pyipmi/utils.py
Outdated
| raise CompletionCodeError( | ||
| rsp.completion_code, | ||
| cmdid=rsp.cmdid, | ||
| netfn=rsp.netfn, |
There was a problem hiding this comment.
I think doing 'cmdid=rsp.cmdid & 0xfe' would be a better solution than adding the 1 in the dict. What do you think?
There was a problem hiding this comment.
Yes seems good to me
Signed-off-by: canteuni <21243338+canteuni@users.noreply.github.com>
Signed-off-by: canteuni <21243338+canteuni@users.noreply.github.com>
canteuni
force-pushed
the
add_cmd_error_code_descr
branch
from
b9d4fe8
to
17decdb
Compare
|
Looks good to me. Thank you! |
This PR aims to add some command-specific completion code descriptions.
Some descriptions were already written but never used, so I add the corresponding dictionary to the
CompletionCodeErrorimplementation. This dictionary also needs the corresponding command ID in order to find the description so I add a reference to the command ID in places wherecheck_completion_codeis called and where I'm sure the specific CC descriptions exist for this command.