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

propose a grouping for the triplet cd-pmd-penalty in transponder #35

Closed

Conversation

EstherLerouzic
Copy link
Collaborator

@EstherLerouzic EstherLerouzic commented Jun 10, 2020

   |     +--ro max-chromatic-dispersion                         decimal64
   |     +--ro max-polarization-mode-dispersion                 decimal64
   |     +--ro chromatic-and-polarization-dispersion-penalty* []
   |        +--ro chromatic-dispersion            decimal64
   |        +--ro polarization-mode-dispersion    decimal64
   |        +--ro penalty                         decimal64

Signed-off-by: EstherLerouzic esther.lerouzic@orange.com

@EstherLerouzic
Copy link
Collaborator Author

The grouping is in the previous commit. I update the PR merging the two commits, it will be clearer

Adding a madatory attribute for max values for CD and PMD

penalty is mandatory (otherwise an element in the list is meaningless)

       |     +--ro max-chromatic-dispersion                         decimal64
       |     +--ro max-polarization-mode-dispersion                 decimal64
       |     +--ro chromatic-and-polarization-dispersion-penalty* []
       |        +--ro chromatic-dispersion            decimal64
       |        +--ro polarization-mode-dispersion    decimal64
       |        +--ro penalty                         decimal64

Signed-off-by: EstherLerouzic <esther.lerouzic@orange.com>
@sergiobelotti
Copy link
Collaborator

I think the change is fine. We need to add a correct description,

Copy link
Member

@italobusi italobusi left a comment

Choose a reason for hiding this comment

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

Few minor comments:

  1. indentation of the code needs some clean-up

Could be addressed before or after merging the PR.

I can make a proposal if you like/prefer.

  1. need to add some references for the definitions

This is a general comment applicable to the whole document we have got from CCAMP WG. We can address it at a later stage

@sergiobelotti
Copy link
Collaborator

Few minor comments:

  1. indentation of the code needs some clean-up

Could be addressed before or after merging the PR.

I can make a proposal if you like/prefer.

  1. need to add some references for the definitions

This is a general comment applicable to the whole document we have got from CCAMP WG. We can address it at a later stage

To move ahead, could you make you a proposal and then we can merge the code.

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