Skip to content

Conversation

@thesummer
Copy link
Contributor

According to to CiA 306 version 1.3.0 Section 5.3.3.3 Network variable values the field name is Baudrate (with small r)

@acolomb
Copy link
Member

acolomb commented Feb 23, 2023

Looks good in principle, however we should think about a migration plan or something. Users relying on the previous spelling might have existing EDS files stop working, no? Or is the whole parsing case-insensitive?

@thesummer
Copy link
Contributor Author

thesummer commented Feb 24, 2023

Looks good in principle, however we should think about a migration plan or something. Users relying on the previous spelling might have existing EDS files stop working, no?

EDS files should be fine. This only affects DCF files (as these have the DeviceComissioning section). The current behavior is that a standard conformant DCF file will cause a runtime error.

Or is the whole parsing case-insensitive?

That I don't know. This library at least is case sensitive.

@thesummer
Copy link
Contributor Author

Just checked the standard:

The keyname is not case sensitive.

@acolomb
Copy link
Member

acolomb commented Feb 25, 2023

Then could you maybe check the code and make sure it is handled case-insensitive here as well, as part of the PR?

@thesummer
Copy link
Contributor Author

@acolomb I had a short look at it. What I don't understand is why this error happens at all. Accoridng to the docu of configparser the keys should be read case insensitive, but here: https://github.com/christiansandberg/canopen/blob/master/canopen/objectdictionary/eds.py#L90 the case of the key seems to matter.

Do you have any suggestion for this. I am not that familiar with the configparser library.

@dirtcrusher
Copy link

I found why the error is happening @thesummer.

By setting the optionxform method of the ConfigParser to the str method here, we explicitly change the ConfigParser to be case sensitive.

(This is even mentioned as an usage example of the optionxform method in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants