-
Notifications
You must be signed in to change notification settings - Fork 190
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
Export Eds or Dcf #254
Export Eds or Dcf #254
Conversation
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.
This will be a nice addition!
We should however stick to PEP-8 coding conventions as much as possible. Variables and attributes should be snake_case and whitespace used consistently.
Ok, I think that is it. I think i processed all of the feedback :) Thanks for the quick review btw 👍 |
I guess this will fix #235 when merged. Should be linked. |
Thanks @acolomb I looked before starting, but i didn't see it. |
I changed it to a WIP, because i am using it in a project now, and i keep finding missing features. So i'll just add those, and put it back. |
Sounds good! There are some camelCase style code left also. |
605ada7
to
06b1454
Compare
The previous one used installed some stuff into usr/local/lib/ next to the intended usr/lib/dist-pakages target. Now using a bdist_wheel package, which is much more clean
Now eds files can also be written. To make the written eds file resemble the read one more, the `DeviceInfo` section is now also read and stored. The `FileInfo` section is also read, but stored privately.
now export_od accepts a filename as destination. If no docType is given, the extension is used as a default. if this fails, eds is still the fallback also better documentation
snake_case in test
06b1454
to
483d4be
Compare
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.
Sorry, missed that this was ready. Looks good, just some minor formatting changes and merge with master and it's good to go I believe.
Co-authored-by: Christian Sandberg <christiansandberg@users.noreply.github.com>
Cool, thanks for the review :) |
also extended the changes in code style (type annotations)
class DeviceInformation: | ||
def __init__(self): | ||
self.allowed_baudrates = set() | ||
self.vendor_name:Optional[str] = None |
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.
Thanks for adding type annotations! However it looks like all attributes are parsed as strings from the EDS so either let them all be strings or convert them during parsing.
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.
Right, very attentive. Fixed it now.
They used to be all strings copied straight from the eds file. Now they are str, int and bool
I needed this for work, so i wrote it. Also added a unit test.
This fixes #235
supported
not supported
compactSubObj
sectionsPs. I improved the deb script slightly. (My previous pull request) So i just included it.
I could rebase it out if we want to be all proper like that :)