sonic-platform-common
sonic-platform-common copied to clipboard
[sonic_eeprom] Class methods shouldn't take the EEPROM data as a parameter
EEPROM class should obtain EEPROM data when instantiated and store as a class member, then use that data in methods rather than having to pass the raw data into the methods. It defeats the purpose of object-orientation.
Hi, I'd like to take this issue. It's a requirement of my course and therefore, I'll make sure to resolve it. Can you please assign it to me?
@Piyushagg19: Thank you for volunteering! I have assigned the issue to you. If you have any questions along the way, please feel free to discuss here.
@jleveque: I went through the 'eeprom_base' file but I'm not sure if I understand the issue completely. Can you please explain the issue a bit more or provide me with an example. Also, if you could direct me to the file that needs the necessary changes, it would be great.
@Piyushagg19: You'll notice that the open_eeprom()
method returns a file handle to the EEPROM, which is in turn passed back into most of the methods in the class. It would be better if open_eeprom()
returned a boolean value and stored the open file handle internally in the class, preventing the need for the caller to store it and pass it back in to other methods.
@jleveque: Hi. Are there any test scripts available that I can run to verify the changes made? There are no instructions in the contributing guide regarding tests.
@Piyushagg19: You should be able to use the decode-syseeprom
utility for testing your changes.
@jleveque: Can you please guide me on how to use it? This is new for me. Thanks!
Log into a SONiC device and run sudo decode-syseeprom --help
:
admin@sonic~$ sudo decode-syseeprom --help
Usage: /usr/local/bin/decode-syseeprom [-s][-m]
Options:
-h, --help show this help message and exit
-d print eeprom from database
-s print device serial number/service tag
-p print the device product name
-m print the base mac address for management interfaces
Running sudo decode-syseeprom
will read info directly from the EEPROM, sudo decode-syseeprom -d
will read cached EEPROM data from our Redis DB.
@jleveque: I'm afraid I don't have access to SONiC device. Can this utility test be performed online or on a virtual machine using Windows machine and qemu or any other way possible?
Unfortunately, a virtual switch doesn't have an EEPROM, and we currently are not mocking one, so you would need access to a physical switch to work on this.
@jleveque: I have created a Pull Request and it has passed all the checks. Can you please let me know how to propagate with the review process?