newrelic / video-agent-roku

New Relic Agent for Roku
Apache License 2.0
18 stars 18 forks source link

Remove `roDeviceInfo.GetVersion()` support #57

Closed john-george closed 1 year ago

john-george commented 1 year ago

GetVersion() API call is present in latest version (v8.2.1 as of today):

function nrGetOSVersion(dev as Object) as Object
    if FindMemberFunction(dev, "GetOSVersion") <> Invalid
        verDict = dev.GetOsVersion()
        return {version: verDict.major + "." + verDict.minor + "." + verDict.revision, build: verDict.build}
    else
        verStr = dev.GetVersion()
        return {version: verStr.Mid(2, 3) + "." + verStr.Mid(5, 1), build: verStr.Mid(8, 4)}
    end if
end function

https://github.com/newrelic/video-agent-roku/blob/master/components/NewRelicAgent/NRAgent.brs#L1230

However, this is resulting in the app failing Roku Certification during static analysis:

------------------------------------------------------------------------------------------------------------------------
-                                                   Deprecated APIs                                                    -
------------------------------------------------------------------------------------------------------------------------
[WARNING] The GetVersion() API has been deprecated. Use the roDeviceInfo.GetOsVersion() function instead, which was introduced in Roku 9.2 OS, to get the major, minor, revision, and build numbers of the Roku OS running on a device. 
Path: /components/vendor/newrelic/NRAgent.brs. Line: 1230. Documentation: https://developer.roku.com/docs/references/brightscript/interfaces/ifdeviceinfo.md#getosversion-as-object.

Please consider removing GetVersion() call altogether and raising minimum supported firmware from 8.2 (according to https://github.com/newrelic/video-agent-roku/blob/master/source/NewRelicAgent.brs#L4) to 9.2.

Happy to create a PR with this change, if the proposal is acceptable!

Note: I see similar issue was already proposed and closed in https://github.com/newrelic/video-agent-roku/issues/52; but this issue was raised to address the actual removal of GetVersion() rather than supporting it and GetOSVersion() simultaneously.