botblox/botblox-manager-software

Universal Command Line Interface for programming managed BotBlox offerings

botblox-manager-software's Issues

Configure the CLI to be more user friendly

Currently, we rely on the user knowing what inputs to put into the optional arguments in the command line. Whilst it is fairly straightforward, it is not idiot-proof. A better idea would be to implement something along the lines of https://github.com/CITGuru/PyInquirer to make an interactive user interface that limits the options the user can pick.

Add test to check single port groups

Test botblox -D /dev/ttyUSB0 vlan --group 1 --group 2 --group 3 --group 4 --group 5 to ensure that we get the expected result (which is that each register setting should be different with only the bitmask for the port being included in the port vlan member group.

Add PR title listing to ensure merge commits to main branch

Issue:

• Right now, commits are not using the conventional commit format when merging from a feature branch to the main branch. This means that it is difficult to keep track of the changes in each version.
• Create a new github action that lints the title of PRs to ensure that it follows the conventional commit format

Acceptance:

• PR titles must be linted by the github action such that the CI checks fail (thereby preventing a merge)
• Can you commit to main branch and then release a new tagged commit?
• Is the readme explaining this?

Add Port mirroring configuration

Follow the format for VLAN configuration and do something similar with port mirroring. Crucially the CLI should have a --reset option for this functionality as well.

Other technical debts are handled in other tickets but try to do this ticket in a way that makes refactoring the code a lot easier when we come to pay off the technical debt.

Add workflow to confluence

Update the confluence wiki with information on how a user runs our application and documentation on what each function on the CLI does.

Includes pages for each functionality (i.e. page for VLAN, port mirroring, etc) and a list of acceptable values

Update the Readme with some of this information (not all of it else the readme becomes way too long).

EEPROM details

In firmware, the number of "commands" that can be stored is set to 20:

https://github.com/botblox/botblox-manager-firmware/blob/e24f68fe2a5a13b4a7480564522f087f9c2ed661/Core/Src/main.c#L43

Is it possible to raise this number if required? I see the internal EEPROM has 512 B, which would allow storing 128 commands (assuming "command" is the 4-byte register write thing). What would be the consequences? Would the startup/memory store/read times get longer?

Also, what is the expected lifetime of the EEPROM? How many writes should it survive?

Run a basic cli in a docker container

This is a necessary part of the CI/CD flow to enable developers regardless of their setup to use our software

Configure pyserial to send serial commands to hardware devices

• Crucially it must do this inside the container shell when the docker container is running. Obviously, a docker container is logically isolated from the hardware so it will be a unique challenge to give the docker container the correct privileges to allow it to send serial data to a hardware device connected on a USB port.

Update `mirror` command with new class structure

Issue:

• Following 64c5c89, try to roll out the corresponding change to mirror command

Acceptance:

• Unit tests will not change (at least in outcome)
• Attempt to keep any refactoring to a minimum

Add proper linting to project

Improve docs

https://botblox.atlassian.net/wiki/spaces/HARDWARE/pages/34373710/BotBlox+Manager+configuration+documentation is talking about portmirror command, but the latest master branch has mirror command.

Also, the Example column contents are weird. Now it contains:

<Base command> <Base positional argument {vlan}> --group 1 2 --group 3 4

Why not

botblox vlan --group 1 2 --group 3 4

?

Refactor python files into modular directory

Self-explanatory title description here - the idea is that it will help with modularising the application further and cleaning up the code base

Make Register class protocol-agnostic

Issue:

• Currently, the issue is that the Register class in switch.py is very specific to SMI protocol, which may not be sustainable going into the future as we are likely to add more devices that communicate with the switch chip.
• This needs to change when we add more switch chips with different protocols

Support Python 3.6

The tool currently requires Python 3.7 or even 3.8. The readme of this package states 3.6+ is required. And I really vouch for supporting 3.6 as that is the default version on Ubuntu 18.04 which still has 2 years of support ahead.

Also, the readme says the installation command is pip install botblox. This command is almost never correct (except when used in a virtualenv, which is however described after this command). On some systems, pip will denote the Python 2 pip. These systems know Python 3's pip as pip3. To avoid any confusion, the safest way to always get Python3 pip is python3 -m pip. Also, I would suggest to either prepend the command with sudo, add the --user or move the virtualenv specs directly to the installation section.

This is what I get running on Python 3.6:

$ python3.6 -m manager
Traceback (most recent call last):
  File "/usr/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/peci1/.local/lib/python3.6/site-packages/manager/__main__.py", line 12, in <module>
    main()
  File "/home/peci1/.local/lib/python3.6/site-packages/manager/__main__.py", line 8, in main
    cli()
  File "/home/peci1/.local/lib/python3.6/site-packages/manager/cli.py", line 82, in cli
    required=True,
  File "/usr/lib/python3.6/argparse.py", line 1716, in add_subparsers
    action = parsers_class(option_strings=[], **kwargs)

Switch not switching after setting up port-based VLAN

I've set up port-based VLAN using the following command:

python3.8 -m manager -D /dev/ttyACM0 vlan --group 1 2 --group 3 4 5

After this command, cables connected to ports 1 and 2 are not able to communicate. When I reset the vlan config, the communication is without problems. I also tried other ports, but it seems that setting any vlan config, all ports that are part of any vlan group are not able to communicate. Their LEDs blink, but no data pass through the switch.

I'll try to investigate this more in depth.

Disable the switch while EEPROM is loading?

I'm not sure if it requires some changes in the firmware or not, but it would be great if the switch were not working in the half-initialized state when it is loading the configuration command-by-command. Some the intermediate states may be invalid and destructive for the rest of the network.

Maybe the 0.11 Power Down register could be used to shut down all ports, then do the configuration changes, and then enable the ports again?

Refactor the datasheet dictionary for vlan configuration as a json file

Currently, the dictionary which contains the register information for the memory registers that we write to on the IC chip is hardcoded in the vlan_config.py as a dictionary. This makes it pretty much unable to be reused.

We want a way to be able to define all the configurations (preferably) in a single config file (.json file) so that it can be used and edited easily, without the need to hardcode into the .py files

Units tests for port mirroring configuration

• Ignore the test for the CLI for now and give it inputs that you would expect from a working CLI.
• Ensure that it creates commands with basic format (check it is a list of lists, check each sublist is full of ints and length 4, check that phy and reg are sensible values, check that there is the stop condition and check that the commands in total equal to expected (i.e. calculated by hand) value.

Add commitizen to add proper changeling

Issue:

• We don't really have a consistent way of making packages that follow the semver convention
• This isn't scalable and adds a lot of pain in ensuring that users are aware of what is in each version

Acceptance:

• Add commitizen and use it to release a package version
• Add instructions on releasing a new version in the README.

Add Readme

Follow similar procedure as in https://github.com/botblox/botblox-manager-firmware

• Take a template readme and fill in the blanks for this repo
• Put installation instructions on the front
• Put information relevant to developers who want to contribute
• Workflow information (make an issue, label it, assign it if necessary, when making a PR link to the issue that it combats, no work is done without a ticket, make sure commit messages follow convention as set out in https://www.conventionalcommits.org/en/v1.0.0-beta.2/#summary and python convention as set out by https://namingconvention.org/python/ (readable form of PEP-8 docs)
• The usual acknowledgements and disclaimer things

Unit tests for VLAN configuration

Follow convention set by #21

Refactor VLAN and port mirroring configuration to inherit from a base classq

Many of the methods used to create the VLAN and port mirroring commands are the same, with slight differences. Is there a way of factoring out the common methods into a base class to improve readability.

Depends on #17 and #13

How does the "configuration commands" storage work?

I haven't found in docs whether all commands I issue via the botblox tool are somehow "stacked" and all stored in the EEPROM, or if always just the last vlan and last mirror commands are stored and reloaded after boot.

Add VLAN CLI functionality

Currently, we are hardcoding the information in that we sent to the firmware on the STM32.

To realise this project and take it to the next level, we need to implement CLI functionality as a high priority.

Start with VLAN as it is relatively straightforward conceptually to think about.

Tasks:

• Think about the architecture of the CLI. I think (without testing it to back it up!) that a CLI with the functionality of <python app.py vlan --groups 1,2,3 4,5> when ran inside the docker container as it is running. vlan is a required positional argument and --groups are the exact groups with space separators between groups and comma delimiters between different members of that group.
• Will need to have a helper function for VLAN that converts the input into the bytes that actually need to be sent to the firmware.
• I think we can hold off on modularisation of the different helper functions vs. cli for now, we just want the implementation working at this stage. Else this ticket will be far too large

QoL: can we remove explicit casts in `tagvlan.py`?

Disclaimer: this may not lead to a desirable and readable outcome and in that case where it becomes a time-sink, I suggest that you don't sink too much time into it. It could become a 'hobgoblin of little minds' as Guido might say.

Issue:

• The casts in tagvlan.py are jarring as I really don't think tagvlan.py needs to know the specifics of the implementation of the _switch.fields attribute.
• The reason why it was necessary in the first place was the PyCharm and presumably other major IDEs would've thrown warnings when trying to call methods but it could not infer the correct type.

Potential solutions:

• One solution to this typing problem is to use a TypedDict mapping type for _switch.fields? Pros: very explicitly and it keeps all the types in one place (complexity in only a single place is better). Cons: it does defeat the point of duck-typing, may as well code in C++, potentially un-pythonic.
• Have a generic set_value and get_value methods which will check the type of the field and then use the correct method accordingly. Pros: achieves the issue and more pythonic than above. Cons: doesn't really expose the types of each field to the outside, just handles the setting and getting of a field value.
• Remove the types and use # type: ignore statements and wrap in try-catch: Pros: Probably the most pythonic. Cons: It could lead to a lot of code with very minimal benefit to the reader.
• Keep as is!

Refactor Port mirroring configuration to use OOP

Use a class with class methods to create the port mirroring configuration

Fix readme shield

Fix pre commit shield not working as expected in readme.

Rename the python module

Python module called manager is quite a generic name. Could you please rename it to something that is less prone to name clashes with other packages?

Set up other Github actions

There are some cool Github actions that we can use to improve the user-friendliness of this repo. Investigate the best actions and see if we can apply a couple to this repo without hurting functionality too much.

Implement basic CI pipeline

When unit tests are complete, we want a way to run them automatically.

Locally:

• Set up a dockerfile and various build scripts to automate the running of tests and flake8

CircleCI:

• Integrate CircleCI with this repo to set up automatic unit tests and flake8.

Implement python packaging tool

The endgame for this project is to make it an installable python package.

Preferably, we want the user to install pip install botblox and it will install the package to a virtual environment (or globally) and one can run commands without any use of Docker (which whilst it ensures portability between host OS, it adds overhead and is one more dependency that the user will have to install on their system).

Milestones:

• Package it into an installer file and use pip install and pip uninstall to check that it can be installed and uninstalled.
• Ensure that you can parse command line arguments (either inline or with a nice multiple choice interface, whichever is implemented at the time this ticket is in progress)
• Use a setup file where a user can configure the device name (they can do this on the command line should it not be mentioned in that config file)
• Figure out how to version this automatically (if possible) using github actions and publish it automatically to PyPi

Can't create bi-port VLAN group

botblox --device {device} vlan --group 1 5 --group 2 5 --group 3 5 --group 4 5 does not seem to work as intended for port 1?

1. Replicate the issue on local setup
2. Investigate what the data should look like vs. what it is
3. Can the ethernet chip only do a certain number of VLANs?

Refactor VLAN configuration to use OOP

Use a class with methods to create the VLAN configuration - will futureproof for reusability in the future