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

WIP for Extended Reports #60

Closed
wants to merge 2 commits into from

Conversation

somersbmatthews
Copy link

@somersbmatthews somersbmatthews commented Jul 28, 2020

I wanted to show that I am working on the Extended Reports.

I am going to work on the tests in extended_reports_test.go, then complete the unmarshal() and marshal() functions in extended_reports.go.

I think the embedded struct idea with pointer struct fields type ExtendedReports struct is a little hacky, but I wanted to leave the rest of the package code as untouched as possible. The different block types do not have different packet types, so header.go and packet.go would not be able to use h.Type to see the different block types.

I could refactor the code in other files to eliminate the need for the embedded struct.

I will format the code at the end when tests are passing and remove some comments.

I don't know if the extended_reports.go file is too large. The extended_reports_test.go file might end up being very large as well. I can parse these files out into different files and/or make new directory with new package name (e.g. package xr)

First thing tomorrow night I'm going to use wireshark to get packet dump from a linphone call (which is supposed to be unencrypted) to get example data for tests. I have never done that, so if there is a better way, please comment below. If you want to help out, please comment below on what you want to work on.

I could be going about all this the wrong way or this issue is outdated and there is no longer need or want for extended reports. If someone could confirm extended reports is still a good idea, that would be great.

Thanks :)

#8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant