Repository: botblox/botblox-manager-software

Issue:
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.

Discussion:
peci1: Also, the docs on https://botblox.atlassian.net/wiki/spaces/HARDWARE/pages/34373710/BotBlox+Manager+configuration+documentation are wrong. Ports not assigned to any group form a separate group - they are not part of all groups. Testing with `--group 1 2 --group 3 4`, port 5 cannot talk to any other port. It is not really clear from the datasheet, but that's the practical outcome I observed. I translate it such that two ports are allowed to communicate iff the bit AND of their `PBV_MEMBER_P*` contains ones at positions corresponding to the two ports. One is not enough.
AaronElijah: Nice one Martin - thanks for spotting that, that's very good of you to help out.

I've confirmed your results from testing and it works as expected. Apologies for the slow testing, my attention has been divided on BotBlox recently but I am interested in implementing many of the issues that you've raised. 

I am happy to merge and approve your PR - you are correct that the docs are outdated in places (maintaining documentation in multiple places does have its risks).

Patch:
diff --git a/manager/cli.py b/manager/cli.py
index 75a07e1..446b919 100644
--- a/manager/cli.py
+++ b/manager/cli.py
@@ -87,7 +87,7 @@ def create_parser() -> argparse.ArgumentParser:
         'vlan',
         help='Configure the ports to be in VLAN groups',
     )
-    vlan_parser_group = vlan_parser.add_mutually_exclusive_group()
+    vlan_parser_group = vlan_parser.add_mutually_exclusive_group(required=True)
     vlan_parser_group.add_argument(
         '-g',
         '--group',
@@ -97,8 +97,11 @@ def create_parser() -> argparse.ArgumentParser:
         choices=[1, 2, 3, 4, 5],
         required=False,
         help='''Define the VLAN member groups using port number,
-        i.e. --group 1 2 --group 3 4 puts makes Group A have
-        ports 1 and 2, and Group B have ports 3 and 4'''
+        i.e. --group 1 2 --group 3 4 makes Group A have
+        ports 1 and 2 and Group B have ports 3 and 4. All unmentioned
+        ports are assigned to default group. If a group has only 1 port,
+        the port gets isolated. In this example, port 5 would
+        not be allowed to communicate with any other port.'''
     )
     vlan_parser_group.add_argument(
         '-r',
diff --git a/manager/data_manager/vlan.py b/manager/data_manager/vlan.py
index fcd52bc..3a5a353 100644
--- a/manager/data_manager/vlan.py
+++ b/manager/data_manager/vlan.py
@@ -19,7 +19,7 @@ class VlanConfig:
             'sys_default': 0xFF,
             'data': 0,
             'choice_mapping': {
-                1: 0b00000010,
+                1: 0b00000100,
             }
         },
         2: {
@@ -31,7 +31,7 @@ class VlanConfig:
             'sys_default': 0xFF,
             'data': 0,
             'choice_mapping': {
-                2: 0b00000100,
+                2: 0b00001000,
             }
         },
         3: {
@@ -43,7 +43,7 @@ class VlanConfig:
             'sys_default': 0xFF,
             'data': 0,
             'choice_mapping': {
-                3: 0b00001000,
+                3: 0b00010000,
             }
         },
         4: {


Test patch:
diff --git a/tests/vlan/test_set_groups.py b/tests/vlan/test_set_groups.py
index 6a6a89c..ac1bb76 100644
--- a/tests/vlan/test_set_groups.py
+++ b/tests/vlan/test_set_groups.py
@@ -8,7 +8,7 @@ from typing import (
 )
 
 
-class TestSetRxMode:
+class TestSetGroups:
     package: List[str] = ['botblox']
     base_args: List[str] = [
         '--device',
@@ -45,7 +45,7 @@ class TestSetRxMode:
         cli_status_code: int = subprocess.call(command)
         assert cli_status_code > 0, 'The command did not exit with an error code'
 
-    def test_single_rx_port(
+    def test_2x2_groups_1_isolated(
         self,
         parser: ArgumentParser,
     ) -> None:
@@ -61,5 +61,5 @@ class TestSetRxMode:
         data = self._get_data_from_cli_args(parser=parser, args=args)
         self._assert_data_is_correct_type(data=data)
 
-        expected_result = [[23, 16, 6, 6], [23, 17, 72, 0], [23, 18, 72, 255]]
+        expected_result = [[23, 16, 12, 12], [23, 17, 80, 0], [23, 18, 80, 255]]
         assert data == expected_result