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

An issue in pdu_encoding #12

Closed
farirat opened this issue Aug 28, 2012 · 4 comments
Closed

An issue in pdu_encoding #12

farirat opened this issue Aug 28, 2012 · 4 comments

Comments

@farirat
Copy link

farirat commented Aug 28, 2012

at line 427:

    def _encodeSchemeDataAsInt(self, dataCoding):
        if dataCoding.scheme == pdu_types.DataCodingScheme.GSM_MESSAGE_CLASS:
            return self._encodeGsmMsgSchemeDataAsInt(dataCoding)
        raise ValueError("Unknown data coding scheme %s" % scheme)

first, scheme is not defined so this code would lead to an error; second the test at line 425:

        if dataCoding.scheme == pdu_types.DataCodingScheme.GSM_MESSAGE_CLASS:

should be fixed with casting values with str() to get the test working.

I didnt push a request with this correction because i didnt understand the difference between _encodeSchemeDataAsInt() method and this one (just on top of it):

    def _encodeSchemeNameAsInt(self, dataCoding):
        schemeName = str(dataCoding.scheme)
        if schemeName not in constants.data_coding_scheme_name_map:
            raise ValueError("Unknown data_coding scheme name %s" % schemeName)
        return constants.data_coding_scheme_name_map[schemeName]

schemeName is casted before the "if" test the right way, so is there any reason why its implemented in a different way in _encodeSchemeDataAsInt()

@theduderog
Copy link
Contributor

Sorry if this is short. I'm on mobile right now. The type for dataCoding.scheme is intended to be an enum, not a string so the "if" test is checking enum equality using the enum module.

I hope this makes sense. If not, can u give sample code or test where it fails?

Sent from my iPhone

On Aug 28, 2012, at 12:19 PM, Fourat Zouari notifications@github.com wrote:

at line 427:

def _encodeSchemeDataAsInt(self, dataCoding):
    if dataCoding.scheme == pdu_types.DataCodingScheme.GSM_MESSAGE_CLASS:
        return self._encodeGsmMsgSchemeDataAsInt(dataCoding)
    raise ValueError("Unknown data coding scheme %s" % scheme)

first, scheme is not defined so this code would lead to an error; second the test at line 425:

    if dataCoding.scheme == pdu_types.DataCodingScheme.GSM_MESSAGE_CLASS:

should be fixed with casting values with str() to get the test working.

I didnt push a request with this correction because i didnt understand the difference between _encodeSchemeDataAsInt() method and this one (just on top of it):

def _encodeSchemeNameAsInt(self, dataCoding):
    schemeName = str(dataCoding.scheme)
    if schemeName not in constants.data_coding_scheme_name_map:
        raise ValueError("Unknown data_coding scheme name %s" % schemeName)
    return constants.data_coding_scheme_name_map[schemeName]

schemeName is casted before the "if" test the right way, so is there any reason why its implemented in a different way in _encodeSchemeDataAsInt()


Reply to this email directly or view it on GitHub.

@farirat
Copy link
Author

farirat commented Aug 29, 2012

Thank you, that make sense now.

What about the scheme variable at line 427 ?

@theduderog
Copy link
Contributor

That looks like a bug. Thanks.

@theduderog
Copy link
Contributor

Fixed. Thx.

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

No branches or pull requests

2 participants