Check for ValueError exception when reading PDOMapping entrys from ED…#386
Check for ValueError exception when reading PDOMapping entrys from ED…#386Obsnold wants to merge 2 commits intocanopen-python:masterfrom
Conversation
acolomb
left a comment
acolomb
left a comment
There was a problem hiding this comment.
I think this possibly hides real errors? Like if someone writes PDOMapping=yes
Wouldn't it be better to check explicitly for an empty string and convert it to zero?
|
Sure that sounds good to me, I'll try and update ti today. |
…y string, this way we don't mask any other errors
|
Alright I've updated it, please take another look. |
|
@acolomb have you had time to look over the changes? |
|
|
||
| pdo_mappable_string = eds.get(section, "PDOMapping", fallback="0") | ||
| if pdo_mappable_string != "": | ||
| var.pdo_mappable = bool(int(pdo_mappable_string, 0)) |
There was a problem hiding this comment.
I guess it is still possible to have invalid input.
Maybe something like this?
var.pdo_mappable = eds.get(section, "PDOMapping", fallback="0") not in ("", "0")There was a problem hiding this comment.
I think we want the invalid input to throw an exception so real errors are caught, though I guess it's a question whether PDOMapping= is valid input or not?
I no longer have access to the EDS files that were causing the issue so I don't have anything to test against.
If I have time I'll try and poke around to see if I can find any other files that exhibit this issue.
it does make me wonder if the handling of invalid input on the other entries should also throw an error?
3 participants
…S files.
Resolves the following issue: