Add O-RAN CCC service model - #392
Conversation
|
test this please |
3 similar comments
|
test this please |
|
test this please |
|
test this please |
|
test this please |
|
Will review it tomorrow - after coming back from holiday off. |
Thanks Woojoong! |
eroshiva
left a comment
eroshiva
left a comment
There was a problem hiding this comment.
Please add unit tests to all top-level PDUs and exclude .asn1 files from the PR
There was a problem hiding this comment.
This file should be excluded from the PR
There was a problem hiding this comment.
Ok, will exclude it. Can you please explain why ASN files are excluded?
There was a problem hiding this comment.
It's an intellectual property of O-RAN. It is not related to ONF.
We've been following this guideline from the beginning of the project and this is the reason why all others SMs do not contain corresponding ASN1 files.
There was a problem hiding this comment.
Ok, Understood. However, I would like to indicate that I created the e2sm_ccc.asn1 files because the one from O-RAN is given in JSON format and I created the ASN file to use the ONF tools for the creation of proto/go files
There was a problem hiding this comment.
Same - this file should be excluded from the PR
* added GHA for ccc service model * Splitted the pdubuilder into top level messeges and add unit tests
eroshiva
left a comment
eroshiva
left a comment
There was a problem hiding this comment.
Thank you for pushing it further. Nice job! Few adjustments left and this PR is good to go!
| result, err := CreateE2SmCCcRIcactionDefinition(ricStyleType, actionDefinitionFormat) | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, result) |
There was a problem hiding this comment.
It would be nice, if the UT would consist of:
- creating a messahe with PDU builder
- validate the message with
.Validate(); - encode the message with recently implemented (in this PR) encoder;
- decode the message;
- compare both encoded and decoded messages and make sure they're equal.
This is a common structure of all unit tests in this repository. Please, align with it.
This comment applies elsewhere to all top-level PDUs.
| assert.NotNil(t, result) | ||
| } | ||
|
|
||
| func TestCreateListOfRanconfigurationStructuresForAdf(t *testing.T) { |
There was a problem hiding this comment.
For non top-level PDUs, please do the .Validate() check in the UT
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestCreateE2SmCCcRIcactionDefinition(t *testing.T) { |
There was a problem hiding this comment.
| func TestCreateE2SmCCcRIcactionDefinition(t *testing.T) { | |
| func TestCreateE2SmCCCRicactionDefinition(t *testing.T) { |
| msg.RicStyleType = ricStyleType | ||
| msg.ActionDefinitionFormat = actionDefinitionFormat |
There was a problem hiding this comment.
Aren't there generated SetXXX() functions? There are, please use them
| ) | ||
|
|
||
| func CreateE2SmCCcRIcactionDefinition(ricStyleType *e2smcommoniesv1.RicStyleType, actionDefinitionFormat *e2smcccv1.ActionDefinitionFormat) (*e2smcccv1.E2SmCCcRIcactionDefinition, error) { | ||
|
|
There was a problem hiding this comment.
Remove the line - applies elsewhere
* Added GHA for ccc service model * Split the pdubuilder into top level messages and add unit tests * Added encoder and decoder test * Apply suggestions from code review --------- Co-authored-by: gab-arrobo <gabriel.arrobo@intel.com>
5 participants
Note: O-RAN provides this service model in a JSON format but the ONF tools are automated to "create" GO structs from ASN files. So, an ASN file was created based on the JSON file, the O-RAN CCC specification and the 3GPP specs.