General Important Points

  1. Write in a clear and consistent coding style. Ensure Code is well formatted with hard tab for whitespace. Match the spacing for each curly braces.
  2. Code should be self-documenting. Variables should be sensibly named, function names descriptive of their purpose. Reserve comments for places where clarification is valuable
  3. If you have a long piece of code and it's hard to tell what it does, consider splitting it up into several functions whose names will describe what's going on.
  4. Keep your headers clean. Put the absolute minimum required in your headers for your interface to be used (Only declarations). As much possible put the function definitions in source (cpp ) file.
  5. Don't expose your classes' private parts. Keep as much of your classes private as possible. All data members should be private; mark them as such by using a leading ‘m’ to differentiate between class variables to local variables: int mSomeVariable; you can then write the getter/setter functions for this variable as int GetSomeVariable() const; and int& SetSomeVariable( int value );. Avoid friend functions.
  6. Use const wherever possible. All member functions that do not modify their object should be const. If your function takes a reference to an object and does not modify that object, you should be passing a const reference.
  7. Write portable code. Don't use compiler-specific features or depend on long or unsigned types being a particular size. Prefer to use size_t for array indexing, especially when dealing with the STL.
  8. Don't leak memory. Every heap allocation using new should have a corresponding delete. Use shared_ptr as much possible to avoid risk of memory leak.
  9. Avoid repeating the same expression or method call - compiler might be smart enough to optimize, but better to use a local variable to collect results that are used in more than one place
  10. Avoid copy/pasted code snippets, or repeated code.  Usually this points to possible refactoring which will result in simpler, more maintainable code.
  11. Follow the Boy Scout rule - Always check a module in cleaner than when you checked it out
  12. Check and respect compilation warnings!  A strict compiler is your friend.
  13. Please use hard tabs, 4 spaces wide.

Classes in C++

  • C++ can have non-obvious overheads.  Pass by pointer or reference can be a lot cheaper than pass by value especially for big classes/structures or std::strings.
  • Separate file for a Class :  Every Class needs its privacy . One separate header and cpp file to be provided for each Class . File name MUST match with the Class implementation . Prefix “Aamp” to all file names with NO underscore character in the file name and use CamelCase notation. Ex: AampGlobalConfig.cpp
  • Follow the rule of three. If your class needs a non-trivial destructor, you should also either implement or disable the copy constructor and copy assignment operators. (furthermore, if you want your data to have the ability to be moved cheaply, also define the move constructor and move assignment)
  • Don't use global data and global static functions. Instead, encapsulate it in a class and design your interfaces effectively.
  • Use the constructor initializer list. (the grader compiler will force you to, in any case)

Header Dependencies

  • Each header must protect itself from being included more than once, using following convention:

#ifndef __AAMP_GLOBALCONFIG_H__

#define __AAMP_GLOBALCONFIG_H__

..

#endif

  • For any given header/class combination, including the class’s header must be sufficient to use its public APIs. Any dependent headers required as part of public class defintions should be #included from within the main header.  External headers required only as part of internal implementation must be #included from the .cpp file.
  • Avoid mutual dependenies of header files. For example, currently: priv_aamp.h includes `main_aamp.h`.. In the same time, main_aamp.h uses `PrivateInstanceAAMP`; which is declared in the priv_aamp.h

Other

  • Prefer using nullptr to NULL.
  • Associate asterisks and ampersands with the variable name, not data type. Declare your pointers like int *a, *b; and references like int &foo = other;.
  • Do take advantage of the standard library.  Generally, don't try to rewrite data structures and algorithms that have already been implemented well in the standard library unless you have a good reason.
  • Variables that are logically on/off switches should be implemented using bool, rather than int.
  • Implement at most one return statement from any function. this makes code flow more straightforward, and helps avoid human error with cleanup of locally managed resources.
  • Function/method Naming should be implemented using Capital letter followed by camel notation, i.e. FindLineLength . Avoid Special characters (like underscore) from function names and use CamelCase notation.
  • {} are required for blocks  , even if it is single line of code within if condition or for loop.
  • Instead of logprintf use following macros based on the severity of the log as log levels can be limited externally: AAMPLOG_TRACE , AAMPLOG_INFO ,AAMPLOG_WARN , AAMPLOG_ERR
  • When you start work on ticket , move ticket status to Code development . Update Development Owner and Development Manager.  Ensure Sprint label is added (aamp_<sprintnumber>)
  • When closing ticket (duplicate / or any reason) , update triage info with explanation and root cause analysis..
  • Please update ticket with development logs before sharing the review
  • Code review or Ticket must include clear test procedure for QA, not generic "Tested in Xi5/Xi6"
  • Whenever you raise any code review outside AAMP like AVE / XRE / Fog  , make sure you trigger verification build manually . Here is the link https://jenkins.ccp.xcal.tv/jenkins/job/Gerrit-Trigger-Custom-Verification-SingleReview-Pipeline/

Use proper printf format specifiers, i.e.

specifier

type

%d

int
%uunsigned int
%l long
%luunsigned long
%lllong long
%ullunsigned long long
%ffloat
%lfdouble
%zusize_t

PRIu64

for uint64_t


Steps for using Coverity tool

1.     Procure access to Coverity portal
·       Coverity Portal url: https://coverity-wc-a1.cable.comcast.com/login/login.htm
·       Credentials : NTID and password
·       Raise RDKSVREQ ticket for access permission.
·       Sample Request: https://ccp.sys.comcast.net/browse/RDKSVREQ-21961
·       Please note that it takes 24 hrs for the activation to reflect in the Coverity portal
2.     Create gerrit link for the latest code check in
·       Complete the code changes in the dev_sprint.
·       Obtain the patch using the command
--> git format-patch -1
·       Raise  the gerrit , along with the bb file in the required the sprint
              Similar to this -> https://gerrit.teamccp.com/#/c/433297/
3.     Generate Jenkins build with the Gerrit patch set generated as part of step 2 and USE_COVERITY flag checked
4.     Once the build is successful, get the 'snapshot ID ' from the Jenkins build console logs. Just search for the string 'snapshot ID ' in the Console Output -> full log for the Jenkins build.
5.     Login to the coverity portal and navigate to Snapshots->All in project
6.     Will get a list of snapshotid's , select the snapshot id got from build, navigate to the aamp component and check for the new defects that were observed by checking the new defects and the date
7.     Will be listed with the new set of defects that were observed, check whether there is any defects new raised from the current file changes.
8.     Once the bugs identified are fixed, use the same procedure to confirm that the Coverity scan finds the issues fixed.

Doxygen-Friendly Coding Conventions

(following courtesy of Deepan, under review)

Kindly go through the document to understand how to do Doxygen coding convention

AAMP Logging Guidelines

Following log levels are supported in AAMP logging . Need to use respective log macro needed for each level 

While tuning, by default log level is set to (more noisy) INFO level.  Once a given tune is compete, default log level is changed to WARN until end of the playback.  This is compromised to avoid log flooding during steady state.

Log Level

Macro to be used 

Description

ERROR

AAMPLOG_ERR

Log level to print "error" logs.  This is intended to print things that should never (logically) happen, severe problems worth raising tickets.
INFO

AAMPLOG_INFO

Log level to print debug / informative messages . INFO level logs are normally available while tuning, but disabled during playback.  This level logs are good for quick issue triage. 

Developer can force INFO level logging to always be printed using aamp.cfg "info"

TRACE

AAMPLOG_TRACE

Log level to print trace messages . Used for triaging/development purpose only. Never gets logged in production. Trace level logging can be enabled with configuration 
WARN

AAMPLOG_WARN

Log level to print warning messages and for any logs with critical information during regular flow. This log level will be printed always.  Warnings should be used for exceptions that player is able to handle robustly, or for environmental problems that don't reflect a client issue.

Process Notes

  • Please add label aamp_<sprint> (i.e. aamp_2302) to all the tickets being worked this sprint along with _aamp label.
  • Ensure Dev Owner and Dev Manager fields are populated.
  • Story points should  be updated in each ticket before raising review
  • To adhere to Sprint Release timing from Release Management team , we also need to merge all AAMP code fixes to stable2 before sprint end.
  • Development freeze will be done by end of 2nd week followed by a week for regression and validation time and a week for approval and merge to stable2.
  • For complex RDK tickets spanning multiple sprints, separate development branches can be requested.
  • Ticket should be moved from draft/under review to Code development . All bug tickets must capture a Root Cause Analysis.
  • Before requesting merge, please make sure that tests/utests are still working.  We strongly recommend adding new utests to improve code coverage and protect against regressions.
  • When ready with review , add label _AAMP_PENDING_REVIEW.  Make sure development test logs are added to the ticket and update the test scenarios clearly in the ticket.  This is particularly important - need to ensure that test guidance is clear for QA engineers.
  • Link the tickets correctly so that dependency can be picked for cherry picks.
  • Message #aamp-dev-private following convention there.


  • No labels