Msn crtl tester

[AndyC534]

hello

[kchan5071]

1. generally if youre going to have stuff commented out(which you should avoid, but i do the same thing so..) you should explain what it does or why its still there, is it leftover from an old version, is it some sort of switch between systems etc.
   

   robosub_software_2025/test_msn_ctrl.py

   Lines 13 to 18 in e3dbb91

   	# import modules
   	# from modules.pid.pid_interface import PIDInterface
   	# from modules.sensors.a50_dvl.dvl_interface import DVL_Interface
   	# from modules.vision.vision_main import VisionDetection
   	# from socket_send import set_screen
   	# from coinflip_fsm import CoinFlip_FSM

2. just make sure you clean up the tab spacing if youre going to align them, 2 sec fix
   

   robosub_software_2025/test_msn_ctrl.py

   Lines 1 to 9 in e3dbb91

   	from multiprocessing import Process, Value
   	from shared_memory import SharedMemoryWrapper
   	from fsm.test_fsm import Test_FSM
   	from utils.socket_send import set_screen
   	from modules.test_module.test_process import Test_Process
   	from fsm.gate_fsm import Gate_FSM
   	from fsm.slalom_fsm import Slalom_FSM
   	from fsm.octagon_fsm import Octagon_FSM
   	from fsm.return_fsm import Return_FSM

3. some of the functions have docstrings, some don't. I would highly suggest writing them

4. generally in this codebase I have avoided using abbreviations for words, even simple things because autocomplete is a thing and I find that having the extra context (does 'tx' mean target_x or transmit for uart)is nice. even though it can be obvious what it is, it just helps keep stuff off the mental stack and make it more clear at a glance when others are looking at your code. take this with a grain of salt since i tend to be more verbose than most people like being.

if you don't want to do the whole writing full words without abbreviations, that thats fine, but the biggest culprits that do have an issue with variable naming are:

robosub_software_2025/test_msn_ctrl.py

Lines 46 to 47 in e3dbb91

	def approx_equal(a: float, b: float, tol: float = 0.05) -> bool:
	return abs(a - b) <= tol

robosub_software_2025/test_msn_ctrl.py

Line 58 in e3dbb91

m = (mode_name or "").lower()

robosub_software_2025/test_msn_ctrl.py

Line 69 in e3dbb91

s = root.get("slalom", {})

robosub_software_2025/test_msn_ctrl.py

Line 77 in e3dbb91

o = root.get("octagon", {})

robosub_software_2025/test_msn_ctrl.py

Line 84 in e3dbb91

r = root.get("return", {})

6. you're assuming the shared memory variables exist, if they don't for any reason, this will be a silent crash, since your LSP wont pick it up, worth checking to see if the variables exist first
   

   robosub_software_2025/test_msn_ctrl.py

   Lines 60 to 89 in e3dbb91

   	if m == "gate":
   	g = root.get("gate", {})
   	# Prefer the final gate target (x, y, z); fall back to buffers if absent
   	tx = float(g.get("x", g.get("x_buf", 0.0)))
   	ty = float(g.get("y", g.get("y_buf", 0.0)))
   	tz = float(g.get("z", g.get("z_buf", 0.0)))
   	return tx, ty, tz
   	elif m == "slalom":
   	s = root.get("slalom", {})
   	# Use first waypoint as the immediate target, and 'z' for depth
   	tx = float(s.get("x1", s.get("x_buf", 0.0)))
   	ty = float(s.get("y1", s.get("y_buf", 0.0)))
   	tz = float(s.get("z", s.get("z_buf", 0.0)))
   	return tx, ty, tz
   	elif m == "octagon":
   	o = root.get("octagon", {})
   	tx = float(o.get("x", o.get("x_buf", 0.0)))
   	ty = float(o.get("y", o.get("y_buf", 0.0)))
   	tz = float(o.get("z", o.get("z_buf", 0.0)))
   	return tx, ty, tz
   	elif m == "return":
   	r = root.get("return", {})
   	# Head toward first leg; use 'depth' if provided, else z_buf/0
   	tx = float(r.get("x1", r.get("x_buf", 0.0)))
   	ty = float(r.get("y1", r.get("y_buf", 0.0)))
   	tz = float(r.get("depth", r.get("z_buf", 0.0)))
   	return tx, ty, tz

   
   

   robosub_software_2025/test_msn_ctrl.py

   Lines 104 to 106 in e3dbb91

   	print(f"x: {sm.dvl_x.value:.2f} → {sm.target_x.value:.2f}")
   	print(f"y: {sm.dvl_y.value:.2f} → {sm.target_y.value:.2f}")
   	print(f"z: {sm.dvl_z.value:.2f} → {sm.target_z.value:.2f}")

7. i'm not sure why you're copying the shared memory object here, this is (probably) a logical error
   

   robosub_software_2025/test_msn_ctrl.py

   Line 103 in e3dbb91

   sm = shared_memory_object

8. if the file is not there for any reason your stuff will crash, have a default case for the things youre pulling out of the file(if you want you can also wrap it in a try except)
   

   robosub_software_2025/test_msn_ctrl.py

   Lines 131 to 135 in e3dbb91

   	with open(os.path.expanduser("~/robosub_software_2025/objects.yaml"), "r") as file:
   	data = yaml.safe_load(file)
   	course = data['course']
   	test_delay = data[course]['delay']
   	test_mult = data[course]['mult']

@rsunderr, @AustinOwens please add anything you think as well

[AustinOwens]

I know that both you and Kai know the code backwards and forwards, so I'm not code to review what the code does functionally. When it comes to formatting and proper "pythonic" programming, why not run linters and formaters on your python code? I do this all the time at work...in fact, every time I save the file in my editor, it does it for me automatically. It typically runs the following linters and formaters:

• flake8
• pydocstyle
• mypy
• black
• isort

That way you never have to worry about the formatting stuff again. And if the whole software team adopts these (or a subset of these), all code will look the same and it makes reviewing easier in the long run.

Oh, and you can also setup a CI/CD pipeline in your repo that forces compliance to these formaters and linters, and if it's not compliant, the pull request will be automatically flagged with a warning telling the person to fix it before it can be merged to main/master.

[kchan5071]

I know that both you and Kai know the code backwards and forwards, so I'm not code to review what the code does functionally. When it comes to formatting and proper "pythonic" programming, why not run linters and formaters on your python code? I do this all the time at work...in fact, every time I save the file in my editor, it does it for me automatically. It typically runs the following linters and formaters:

• flake8
• pydocstyle
• mypy
• black
• isort

That way you never have to worry about the formatting stuff again. And if the whole software team adopts these (or a subset of these), all code will look the same and it makes reviewing easier in the long run.

Oh, and you can also setup a CI/CD pipeline in your repo that forces compliance to these formaters and linters, and if it's not compliant, the pull request will be automatically flagged with a warning telling the person to fix it before it can be merged to main/master.

I had mixed feelings on this I have definitely gone back and fourth thinking about it. On one hand its obviously very nice to have something always checking your style to make sure you're following a guideline, on the other hand it was another friction point between showing up at the GBM and making their first commit. I don't really have strong feelings either way, but I eventually decided against it at the time because we were trying to make onboarding as simple as possible. Basically it would be nice for those who know what they're doing, but not great for a newbie, and our top priority at the time was getting more people.

[AustinOwens]

Yah, I understand. And I know members are just starting to learn Git for the first time. I can see both sides. Of course, it's ultimately up to you all. You have time before the next competition and I think if any group at SDSU can figure it out, it's Mechatronics.