sonic-platform-common
sonic-platform-common copied to clipboard
[sonic_eeprom/eeprom_base] EEPROM data instantiated and stored as a class member
Description
-
open_eeprom( ): Initially the function returned a file handle which was passed back into most of the methods in the class. Now, the file handle is declared with a global scope and stored internally in the function, there is no need for other functions to first store the file handle and then access it. The function now returns a bool value 'using_eeprom' which indicates whether the file is read from cache or the path specified for the EEPROM file.
-
read_eeprom_bytes( ): The open_eeprom( ) function is referred twice in this function. In the first reference, simply the file handle is passed and accessed. In the second reference, a new bool variable 'eeprom_flag' is defined. Since the open_eeprom( ) function returns the flag 'using_eeprom' indicating whether the path specified for EEPROM is taken or not, it ensures that after removing the 'cache_name' file path from the OS, the file path is set to retrieve the file contents from the path specified for the EEPROM file.
Motivation and Context
" Fixes #170 " Initially, the code was defeating the purpose of object-oriented programming. Now, necessary changes are made to resolve the issue.
How Has This Been Tested?
The testing has been done in the user's local system by trying mimicking the environment and its functionalities.
@jleveque: Can you please review the changes?
This pull request introduces 1 alert when merging 110284342d20ab263908c63ac10a6e73f5461fe2 into 295b68ca4c6ba1b785d5768e36154f1a52479d03 - view on LGTM.com
new alerts:
- 1 for Unused local variable
@jleveque: From what I understood, I modified the code to meet the requirements. Can you please check if the update suffices all the requirements? If not, please let me know the rest of the areas that require changes.
@jleveque: From what I understood, I modified the code to meet the requirements. Can you please check if the update suffices all the requirements? If not, please let me know the rest of the areas that require changes.
All methods which expect the full raw EEPROM bytes to be passed in should also be modified to default the parameter to None
and then check if it is None
, in which case it will use the new class member.
@jleveque: Sorry for pestering you again but this is my first contribution to the open-source community. Hope you understand! I'm not able to understand what methods are you referring to when you say 'methods which expect the full raw EEPROM bytes to be passed in'. If you can please provide me with an example or mention the methods that require the changes, I'll be able to understand better and get the rest of the work done. Thank you.
@jleveque: The update consists of two changes.
- Reverting the
read_eeprom_bytes()
return statement back to as it was. - Using more descriptive names of variables.
The error states -
ERROR: Could not find a version that satisfies the requirement sonic-yang-models>=1.0 (from sonic-config-engine) (from versions: none)
ERROR: No matching distribution found for sonic-yang-models>=1.0
##[error]Bash exited with code '1'.
Is this because of the changes made by me in the eeprom_base.py file or because of some other dependencies issues as the error states?