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

Used to identify the Automatic Tank Gauge protocol #2527

Merged
merged 9 commits into from
Aug 23, 2024
Merged

Conversation

wssxsxxsx
Copy link
Contributor

@wssxsxxsx wssxsxxsx commented Aug 23, 2024

Please sign (check) the below before submitting the Pull Request:

Link to the related issue:

Describe changes:

Copy link
Collaborator

@IvanNardi IvanNardi left a comment

Choose a reason for hiding this comment

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

Some changes:

  • add <ClCompile Include="..\src\lib\protocols\atg.c" /> to windows/nDPI.vcxproj (like you did in previous PR)
  • update results files: NDPI_FORCE_UPDATING_UTESTS_RESULTS=1 ./tests/do.sh and "git add" all changed and new files

@IvanNardi
Copy link
Collaborator

LGTM. @utoni, @koltiradw, @0xA50C1A1 any comment?

@IvanNardi
Copy link
Collaborator

@wssxsxxsx, could you sign the CLA, please?

@wssxsxxsx
Copy link
Contributor Author

@IvanNardi Is it to send the CLA by email? I have already sent it.

@IvanNardi
Copy link
Collaborator

@IvanNardi Is it to send the CLA by email? I have already sent it.

No you don't, but I don't find your GitHub name (wssxsxxsx) in the list of the people who signed it. Could you double check, please?

@wssxsxxsx
Copy link
Contributor Author

@IvanNardi Is it to send the CLA by email? I have already sent it.

No you don't, but I don't find your GitHub name (wssxsxxsx) in the list of the people who signed it. Could you double check, please?

@IvanNardi Is it sent here-> legal@ntop.org

@0xA50C1A1
Copy link
Contributor

LGTM. @utoni, @koltiradw, @0xA50C1A1 any comment?

I think it would be good to add some additional checks, in case this protocol can use a port different from 10001.

Copy link

Copy link
Collaborator

@IvanNardi IvanNardi left a comment

Choose a reason for hiding this comment

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

I pushed some cosmetithic changes.
I think this PR is fine; we can always improve the detection later, if needed

@IvanNardi IvanNardi merged commit 8894ebc into ntop:dev Aug 23, 2024
36 checks passed
@IvanNardi
Copy link
Collaborator

Thanks @wssxsxxsx for your work and your perseverance!
Thanks everyone for the reviews

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