wmnsk / go-pfcp

PFCP (Packet Forwarding Control Protocol) in pure Go.
MIT License
124 stars 54 forks source link

QuotaValidityTime should be processed as duration instead of time #122

Closed mogaika closed 12 months ago

mogaika commented 12 months ago

As in 3GPP TS 29.244 version 16.5.0 Release 16 8.2.132 Quota Validity Time : It contains the quota validity time in seconds. Current implementation parses and creates it as time.Time

mogaika commented 12 months ago

Also Time Threshold currently converted as uint32, but it should be treated as time.Duration. Overall I like uint32 more, because it enforces you to think about rounding to seconds correctly, but for consistency it probably should be time.Duration.

wmnsk commented 12 months ago

Thanks, fixed in #123.

Overall I like uint32 more, because it enforces you to think about rounding to seconds correctly, but for consistency it probably should be time.Duration.

I understand it, and when passing values from a protocol to another this "friendliness" would bother as well. As you may already know, there is ie.New function as a generic constructor for creating anything by hand, but this time I also exported these functions to make manual creation easier (and updated README). For value retrieval, I may add something to return "raw" type or may not - for now I don't think it's worth, but let me know your opinion if any.

// NewUint8ValIE creates a new IE with uint8 value.
func NewUint8IE(itype uint16, v uint8) *IE {
    return newUint8ValIE(itype, v)
}

// NewUint16ValIE creates a new IE with uint16 value.
func NewUint16IE(itype uint16, v uint16) *IE {
    return newUint16ValIE(itype, v)
}

// NewUint32ValIE creates a new IE with uint32 value.
func NewUint32IE(itype uint16, v uint32) *IE {
    return newUint32ValIE(itype, v)
}

// NewUint64ValIE creates a new IE with uint64 value.
func NewUint64IE(itype uint16, v uint64) *IE {
    return newUint64ValIE(itype, v)
}

// NewStringIE creates a new IE with string value.
func NewStringIE(itype uint16, v string) *IE {
    return newStringIE(itype, v)
}

// NewFQDNIE creates a new IE with FQDN value.
func NewFQDNIE(itype uint16, v string) *IE {
    return newFQDNIE(itype, v)
}
mogaika commented 12 months ago

Indeed it is good idea to expose some common methods like these, especially this could help with vendor ies

wmnsk commented 11 months ago

Great, thanks for your feedback. I haven't considered much about vendor-specific IEs so far, but now I'm wondering to add a value retrieving methods like ValueAsUint32() for easier handling of vendor-specific IEs. I guess current users already have such functions on their own, but anyway.