-
Notifications
You must be signed in to change notification settings - Fork 154
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
LTE Location Services: SLh and SLg interfaces implementation #48
Conversation
Find attached the 3GPP specs versions used for this work. 3GPP TS 29.173 v13.0.0 - Location Services (LCS); Diameter-based SLh interface for Control Plane LCS -20151221.docx |
Thanks @FerUy. @brainslog can you review ? |
@brainslog, as agreed today with @deruelle, I pushed this commit after working on these issues on a very long flight back home after a business trip. It's more about saving all the work than asking for peer review at this time, as some few errors and TODOs are pending in a couple of files (as well as whole testsuite and core/jdiameter-ha/... needed files). Therefore, coming back to the initial plan and changing the one commented at the time of the PR one week ago. In other words, now going for the full implementation in this PR. So, again, I pushed this work more for storing matters than for peer reviewing, as still there's some work to do and having other related priorities (also, now the branch has conflicts with master it didn't have 6 days ago)... but we are 5000+ lines of code and 53 files closer than we were yesterday ;) |
@FerUy, thanks for this Pull Request. Not reviewing the content itself yet, as you say it is still a WIP, but I'd ask you not to modify what hasn't been really modified, like whitespace changes, etc. As can be seen in the first commit, the Avp.java has been totally re-written (and although second commit says it tries to fix it, it's still the same). Also, previously we had some mixed indentation, with 2, 4 spaces and at some places even with tabs and now I've cleaned it up to using 2 spaces (thus the conflicts you see now). Also there's a maven checkstyle plugin in place now that will throw failure at build time when the style is not adequate. Please try to fix this two items and let me know when you'll be ready to proceed with peer review. Thanks! |
Thanks @brainslog, I'll let you know when this work is ready for peer review. |
Issue #37 and Issue #38 Update Avp.java There was a problem introduced by automatic checkstyle formatting in Eclipse in line 31 (it added one more line and thus made it appear with so many changes/deletions). Also, made more human readable the comments of reused AVPs either for SLh or SLg. Added SLh and SLg (ELP) AVPs into dictionary, including all applications/commands and AVP's elements definitions according to 3GPP TS 29.172/173. Issue #37 and issue #38 SLh and SLg interfaces implementation initial work after correspondent AVP definition/configuration previous commit. Some few error corrections and TODOs pending. Issues #37 and #38 PR #48 Fixed all compilations errors from previous commits. Added jdiameter-ha classes for both interfaces. Issues #37 and #38. PR #48 Worked on conflicts. Issues #37 and #38. PR #48 Modifications for consistency with latest changes for declaration of attributes in Avp.java class. Issues #37 and #38. PR #48 Started testsuite work. Issues #37 and #38. PR #48 A couple of fixes from previous commit and SLh abstract client and server classes for testsuite implementation start. Issues #37 and #38. PR #48 Attempt to resolve conflicts in Avp.java. Issues #37 and #38. PR #48 Rollback to original Avp.java. Issues #37 and #38. PR #48 Rollback to original Avp.java. fixing 2 spaces instead of 4. Issues #37 and #38. PR #48 Rollback to original Avp.java. fixing 2 spaces instead of 4, 2nd attempt. Issues #37 and #38. PR #48
Issue #37 and Issue #38 Update Avp.java There was a problem introduced by automatic checkstyle formatting in Eclipse in line 31 (it added one more line and thus made it appear with so many changes/deletions). Also, made more human readable the comments of reused AVPs either for SLh or SLg. Added SLh and SLg (ELP) AVPs into dictionary, including all applications/commands and AVP's elements definitions according to 3GPP TS 29.172/173. Issue #37 and issue #38 SLh and SLg interfaces implementation initial work after correspondent AVP definition/configuration previous commit. Some few error corrections and TODOs pending. Issues #37 and #38 PR #48 Fixed all compilations errors from previous commits. Added jdiameter-ha classes for both interfaces. Issues #37 and #38. PR #48 Worked on conflicts. Issues #37 and #38. PR #48 Modifications for consistency with latest changes for declaration of attributes in Avp.java class. Issues #37 and #38. PR #48 Started testsuite work. Issues #37 and #38. PR #48 A couple of fixes from previous commit and SLh abstract client and server classes for testsuite implementation start. Issues #37 and #38. PR #48 Attempt to resolve conflicts in Avp.java. Issues #37 and #38. PR #48 Rollback to original Avp.java. Issues #37 and #38. PR #48 Rollback to original Avp.java. fixing 2 spaces instead of 4. Issues #37 and #38. PR #48 Rollback to original Avp.java. fixing 2 spaces instead of 4, 2nd attempt. Issues #37 and #38. PR #48
With latest version of Wireshark (2.2.3) and by adding all AVP definitions in Wireshark's Diameter dictionary.xml, the attached traces are obtainable, with all AVPs properly populated as for either SLh and SLg specs after running Junit tests (you also need to add TCP ports 13868 and 4868 in Wireshark preferences->protocols-Diameter for this purpose). All AVPs are filled with random but acceptable values. Wireshark's improved dictionary.xml, with AVPs for 3GPP TS 29.172 needed additions, is also attached (latest version only contains a subset of AVPs defined in 3GPP TS 29.172 and 29.173). SLh-SLg_JunitTest_Traces_&_Wireshark_diameter_dictionary.zip |
Closing this PR, which is replaced by PR #111. |
This is a pull request for issues #37 and #38, concerning AVP definition and configuration for LTE location services. Complete interface implementation will follow after this peer review.