Skip to content

Add support for FlexRay symbols #2

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

aamereller
Copy link

The flexray export function does not support flexray frames as of now. The struct and exporter function have been adapted to also support FlexRay symbols.

@kayoub5
Copy link
Member

kayoub5 commented Nov 21, 2024

CI failed

@@ -36,6 +36,7 @@ struct flexray_frame
uint8_t cc;

uint8_t data[254];
uint8_t symbol_length;
Copy link
Member

Choose a reason for hiding this comment

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

In PCAP FlexRay symbol is defined as array of bytes, not just one value

https://www.tcpdump.org/linktypes/LINKTYPE_FLEXRAY.html

@aamereller
Copy link
Author

The site explicitly states that a FR symbol frame has the measurement header of 1 byte and the symbol length of 1 byte, The SL[0..6] values represent a 7-bit unsigned integer (0-127).

@@ -163,6 +163,9 @@ namespace pcapng_exporter {
const uint8_t FR_HDR_SIZE = 7;
uint8_t fr[FR_HDR_SIZE + 254] = { 0 };
fr[0] = (frame.channel << 6) | frame.type;
uint8_t frame_length;
if (frame.type == FR_TYPE_FRAME)
Copy link
Contributor

Choose a reason for hiding this comment

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

@aamereller, could you please fix the indentation inside the if / else blocks ?

@welbouri
Copy link
Contributor

@kayoub5 , could you please re run the CI ?

do you have any other remarks ?

@kayoub5
Copy link
Member

kayoub5 commented Jan 15, 2025

ci failing

@welbouri
Copy link
Contributor

ci failing

I am not seing any pipeline being launched in #3. any explanation ?

I am asking because nothing in the changes looks related to the CI failure.

@kayoub5
Copy link
Member

kayoub5 commented Jan 16, 2025

ci failing

I am not seing any pipeline being launched in #3. any explanation ?

I am asking because nothing in the changes looks related to the CI failure.

most likely because the branch came from a fork

@aamereller
Copy link
Author

Regarding the CI:
Ubuntu-latest now uses Ubuntu 24.04 and g++-13.3, in which either the std::[u]int*_t types are not supported at all or that uses a newer implementation standard than C++-11 which does not support these types. According to the CPPReference site the fixed int types are "supported if, and only if, the implementation directly supports them". Thus the main question for Linux is whether the code should be migrated to comply to the Ubuntu 24.04 standards or the CI shall explicitly continue using Ubuntu 22.04 with which it works.
I can locally reproduce the exact same errors when compiling against Ubuntu 24.04 instead of Ubuntu 22.04.
The Windows CI still seems to refuse to install the necessary libraries via Chocolatey.

@aamereller aamereller changed the title Integrate ANDI-10625 Add support for FlexRay symbols Jan 20, 2025
@welbouri
Copy link
Contributor

@aamereller , concerning the linux part, you can force the image image: ubuntu:22.04 by adding this in the linux step in gitlab-ci.yml after line 18.

For Windows CI, I will have a look and get back to you

@welbouri
Copy link
Contributor

@aamereller , please try to replace this (https://github.com/Technica-Engineering/Technica.Traces.Pcapng.Exporter/blob/main/.github/workflows/cmake.yml#L32) with another altyernative since it seems that this issue is impacting others also in github. I have found different issues when searchin choco install winpcap.

Please have a look on dotpcap/sharppcap#454

@welbouri
Copy link
Contributor

https://conan.io/center/recipes/libpcap?version=1.10.5 , you can have a look if needed to this alternative but it is not tested. (Thanks @kayoub5 for help)

@welbouri
Copy link
Contributor

@aamereller , I think that the issue of CI on windows will persist because it is related tothe install of winpcap. Please refer to the previous suggestion

@welbouri
Copy link
Contributor

welbouri commented Feb 3, 2025

@kayoub5 , CI is passing, do you still have any remark ?

@RayanMRABET
Copy link

Hi @kayoub5, does this pull request look good to you?

@kayoub5
Copy link
Member

kayoub5 commented Feb 24, 2025

Do you already have matching PR for BLF or TECMP?

- name: Install (macos)
if: matrix.os == 'macos-latest'
run: brew install libpcap

Copy link
Member

Choose a reason for hiding this comment

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

Why?

@welbouri
Copy link
Contributor

Do you already have matching PR for BLF or TECMP?

@kayoub5 , I think that those are topics that can be handled separetely in case of need

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.

4 participants