Skip to content
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

Implement support for CAN-FD and container frames #6

Merged
merged 5 commits into from
Jan 18, 2024

Conversation

andlaus
Copy link

@andlaus andlaus commented Jan 12, 2024

this is a direct carryover of the same PR from the old repository. All discussions for the original PR still apply.

Andreas Lauser <[email protected]>, on behalf of MBition GmbH.
Provider Information

decode = message_def.decode(bytes(data), allow_truncated=True, decode_containers=True)
except Exception as e:
log.warning("Error processing CAN message with frame ID: %#x", frame_id, exc_info=True)
log.warning("Error: ", e)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line gives a linter error (i think), duplicate as we already have the line above?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how can this be reproduced locally? on my machine, flake8 passes fine:

$ flake8 .
$

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not reported by flake8, it is pylint that we run from https://github.com/eclipse-kuksa/kuksa-can-provider/blob/main/.github/workflows/can-provider.yml#L122

Installed in CI by

pip3 install --no-cache-dir -r requirements.txt -r requirements-dev.txt

This is how I run it locally:

erik@debian4:~/kuksa-can-provider$ pylint -E dbcfeeder.py dbcfeederlib test
************* Module dbcfeederlib.canreader
dbcfeederlib/canreader.py:98:16: E1205: Too many arguments for logging format string (logging-too-many-args)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, should be fixed now: 6f2151b

@erikbosch
Copy link
Contributor

Thanks for moving the PR. The eclipsefdn/eca check fails, in one of your commits [email protected] is listed as author and that user has no ECA. I do not know if the ECA check is run only once (until it succeeds) and that is the reason why we did not got an error for that commit earlier in the old repo.

Maybe it is possibly for you to edit that commit so that it gets the same author as other commits

commit c3d38eb0b3a4ffe7c746218a5b3e22c4a7ce3ea1 (HEAD -> pr6)
Author: Walz <[email protected]>
Date:   Tue Nov 7 15:30:34 2023 +0100

    Make flake8 happy :-)
    
    Signed-off-by: Alexander Walz <[email protected]>
    Signed-off-by: Andreas Lauser <[email protected]>

commit d3b05c6f55e54a2a4da11434be2e5413a0097c01
Author: Alexander Walz <[email protected]>
Date:   Fri Nov 3 09:53:56 2023 +0100

    Adapt output message for reading the database file
    
    Signed-off-by: Alexander Walz <[email protected]>
    Signed-off-by: Andreas Lauser <[email protected]>

@andlaus
Copy link
Author

andlaus commented Jan 15, 2024

The eclipsefdn/eca check fails, in one of your commits [email protected] is listed as author and that user has no ECA.

oops. git configuration error. fixed...

@erikbosch
Copy link
Contributor

The remaining pre-commit error might be a false positive. I think I have seen a similar problem in another repo and will update dependencies and retry

@andlaus
Copy link
Author

andlaus commented Jan 17, 2024

The remaining pre-commit error might be a false positive. I think I have seen a similar problem in another repo and will update dependencies and retry

yes, seems like flake8 does plays style police even within strings. (this is a bit weird IMO, even more so considering that the flake8 on my computer does not complain about it.) anyway, I've added a space before the x format...

AlexMicWalz and others added 5 commits January 18, 2024 13:06
Signed-off-by: Alexander Walz <[email protected]>
Signed-off-by: Andreas Lauser <[email protected]>
Signed-off-by: Alexander Walz <[email protected]>
Signed-off-by: Andreas Lauser <[email protected]>
Signed-off-by: Alexander Walz <[email protected]>
Signed-off-by: Andreas Lauser <[email protected]>
Signed-off-by: Alexander Walz <[email protected]>
Signed-off-by: Andreas Lauser <[email protected]>
Signed-off-by: Andreas Lauser <[email protected]>
Signed-off-by: Alexander Walz <[email protected]>
@erikbosch erikbosch merged commit e737ddb into eclipse-kuksa:main Jan 18, 2024
7 checks passed
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