mercedes-benz / odxtools

odxtools is a collection of utilities to interact with the diagnostic functionality of automotive electronic control units using python
MIT License
171 stars 70 forks source link

Improve auxiliary file handling #310

Closed andlaus closed 3 months ago

andlaus commented 3 months ago

This PR improves the handling of auxiliary files:

Besides this, this PR first fixes the mksomersaultmodifiedpdx.py example: the somersaultecu has been slightly changed (by adding an auxiliary file for a single-ECU job), and thus the modified somersault ECU needed to be dragged along. Unfortunately the script has been broken for the last few weeks...

Andreas Lauser <andreas.lauser@mercedes-benz.com>, on behalf of MBition GmbH. Provider Information

kayoub5 commented 3 months ago

@andlaus those are 3 unrelated topics in one PR, better to split them

andlaus commented 3 months ago

@andlaus those are 3 unrelated topics in one PR, better to split them

I agree in principle, but the auxiliary file stuff requires the first two patches, while the "provide database via the diaglayer" stuff would be unused without the ProgCode changes. Do you still want it to be split?

kayoub5 commented 3 months ago

@andlaus those are 3 unrelated topics in one PR, better to split them

I agree in principle, but the auxiliary file stuff requires the first two patches, while the "provide database via the diaglayer" stuff would be unused without the ProgCode changes. Do you still want it to be split?

Yes please, the changes are too much to review all at once

andlaus commented 3 months ago

okay, done: cf #311 and #312...

andlaus commented 3 months ago

alright: with #311 and #312 merged, I rebased this on top of the latest master. I fixed two further issues as drive-bys (https://github.com/mercedes-benz/odxtools/pull/310/commits/76364d88355df30348ede7fdf598d0271118f585 and https://github.com/mercedes-benz/odxtools/pull/310/commits/63958a8eb26d85648369c830ab22208a6b710884 ), but I don't think that these warrant a separate PR...

andlaus commented 3 months ago

@kayoub5: do you consider this to be merge-ready?