Investigate On-Robot Diagnostics Through a CLI Interface #3379

Andrewyx · 2024-10-20T22:48:02Z

Flash with bazel run //software/embedded/ansible:run_ansible --platforms=//cc_toolchain:robot --//software/embedded:host_platform=PI -- --playbook deploy_robot_software.yml --hosts 10.42.0.53 --ssh_pass thunderbots

Please fill out the following before requesting review on this PR

Description

Introduces new CLI tool with robot_diagnostics_features to run natively on robots. This feature will be usuable while SSH'ed into the robot and includes simplified commands ported over from the robot_diagnostics GUI.

Testing Done

Resolved Issues

Resolves #3322

Length Justification and Key Files to Review

Review Checklist

It is the reviewers responsibility to also make sure every item here has been covered

Function & Class comments: All function definitions (usually in the .h file) should have a javadoc style comment at the start of them. For examples, see the functions defined in thunderbots/software/geom. Similarly, all classes should have an associated Javadoc comment explaining the purpose of the class.
Remove all commented out code
Remove extra print statements: for example, those just used for testing
Resolve all TODO's: All TODO (or similar) statements should either be completed or associated with a github issue

…into Andrewyx/DiagnosticsCLI

…re into Andrewyx/DiagnosticsCLI

formatting_scripts/dictionary/edited_dictionary.txt

src/software/embedded/ansible/BUILD

src/software/embedded/robot_diagnostics_cli/robot_diagnostics_cli.py

itsarune

looking great! excited to see how this turns out

Mr-Anyone · 2024-11-12T06:39:07Z

fire branch with clean git diff.

Mr-Anyone · 2024-11-12T06:42:19Z

src/MODULE.bazel

 bazel_dep(name = "bazel_skylib", version = "1.5.0")
+bazel_dep(name = "rules_pkg", version = "1.0.1")


You should delete this file. We probably aren't going to move to MODULE.bazel anytime soon.

I remember reading an issues about rules_python and pybind11_protobuf having issues with locally installed toolchain somewhere when using MODULE.

Mr-Anyone · 2024-11-12T06:46:14Z

src/defs.bzl

@@ -0,0 +1,97 @@
+load("@rules_pkg//:providers.bzl", "PackageFilesInfo", "PackageSymlinkInfo", "PackageFilegroupInfo")


interesting.

This could be the next nanpb_proto_library in our codebase:

Software/src/proto/nanopb.bzl

Lines 1 to 212 in 10bded3

load("@rules_proto//proto:defs.bzl", "ProtoInfo")

load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")

# The relative filename of the header file.

_FILENAME = "lib/{dirname}/{filename}"

_PROTO_DIR = "{path}/proto"

_OPTIONS_DIR = "{path}/generator/nanopb"

# Command that copies the source to the destination.

_COPY_COMMAND = "cp {source} {destination}"

# Command that zips files recursively. It enters the output directory first so

# that the zipped path starts at lib/.

_ZIP_COMMAND = "cd {output_dir} && zip -qq -r -u {zip_filename} lib/"

def _nanopb_proto_library_impl(ctx):

# This is the folder we will place all our generation artifacts in

#

# Because we need to generate the `.c` and `.h` files for all the proto files required

# for this library, including all the transitive dependencies, we would like to

# generate these in the same folder as each proto file. However bazel will not

# permit us to modify/produce files outside the folder that this rule is called from.

# We get around this be reproducing the folder structure under a generation folder

# and adding the root of this directory as an include path.

#

generation_folder_name = ctx.attr.name + "_nanopb_gen/"

generated_folder_abs_path = ctx.genfiles_dir.path + "/" + \

ctx.build_file_path[:-len("BUILD")] + generation_folder_name

# Generate import flags for the protobuf compiler so it can find proto files we

# depend on, and a list of proto files to include

all_proto_files = depset()

all_proto_include_dirs = depset()

for dep in ctx.attr.deps:

all_proto_files = depset(

transitive = [all_proto_files, dep[ProtoInfo].transitive_sources],

)

all_proto_include_dirs = depset(

transitive = [all_proto_include_dirs, dep[ProtoInfo].transitive_proto_path],

)

all_proto_hdr_files = []

all_proto_src_files = []

for proto_file in all_proto_files.to_list():

# Generate the `.pb` files using the protobuf compiler

pb_file_name = generation_folder_name + proto_file.path[:-len(".proto")] + ".pb"

pb_file = ctx.actions.declare_file(pb_file_name)

# Create the arguments for the proto compiler to compile the proto file,

# adding all the transitive include directories

proto_compile_args = ["-o", pb_file.path, proto_file.path]

for path in all_proto_include_dirs.to_list():

proto_compile_args += ["-I%s" % path]

ctx.actions.run(

inputs = all_proto_files,

outputs = [pb_file],

arguments = proto_compile_args,

executable = ctx.executable.protoc,

mnemonic = "ProtoCompile",

use_default_shell_env = True,

)

# Generate the equivalent C code using Nanopb

h_out_name = generation_folder_name + proto_file.path[:-len(".proto")] + ".nanopb.h"

c_out_name = generation_folder_name + proto_file.path[:-len(".proto")] + ".nanopb.c"

c_out = ctx.actions.declare_file(c_out_name)

h_out = ctx.actions.declare_file(h_out_name)

ctx.actions.run_shell(

tools = [ctx.executable.nanopb_generator],

inputs = [pb_file],

outputs = [c_out, h_out],

mnemonic = "NanopbGeneration",

command = "%s -e .nanopb %s" % (ctx.executable.nanopb_generator.path, pb_file.path),

)

all_proto_src_files.append(c_out)

all_proto_hdr_files.append(h_out)

cc_toolchain = find_cpp_toolchain(ctx)

feature_configuration = cc_common.configure_features(

ctx = ctx,

cc_toolchain = cc_toolchain,

)

# Get the compilation and linking contexts from all nanopb srcs

nanopb_compilation_contexts = [

label[CcInfo].compilation_context

for label in ctx.attr.nanopb_libs

if label[CcInfo].compilation_context != None

]

nanopb_linking_contexts = [

label[CcInfo].linking_context

for label in ctx.attr.nanopb_libs

if label[CcInfo].linking_context != None

]

(compilation_context, compilation_outputs) = cc_common.compile(

name = "compile_nanopb_outputs",

actions = ctx.actions,

feature_configuration = feature_configuration,

cc_toolchain = cc_toolchain,

srcs = all_proto_src_files,

public_hdrs = all_proto_hdr_files,

includes = [

generated_folder_abs_path,

] + [generated_folder_abs_path + dir for dir in all_proto_include_dirs.to_list()],

compilation_contexts = nanopb_compilation_contexts,

)

(linking_context, linking_outputs) = \

cc_common.create_linking_context_from_compilation_outputs(

name = "link_nanopb_outputs",

compilation_outputs = compilation_outputs,

actions = ctx.actions,

feature_configuration = feature_configuration,

cc_toolchain = cc_toolchain,

linking_contexts = nanopb_linking_contexts,

)

# platformio_* bazel rules require a provider named transitive_zip_files and an output zip file

# these contain all files needed for compilation with platformio.

name = ctx.label.name

commands = []

inputs = all_proto_hdr_files + all_proto_src_files

outputs = []

for hdr_file in all_proto_hdr_files:

dir = _PROTO_DIR.format(path = name)

if "options.nanopb." in hdr_file.basename:

dir = _OPTIONS_DIR.format(path = name)

file = ctx.actions.declare_file(

_FILENAME.format(dirname = dir, filename = hdr_file.basename),

)

outputs.append(file)

commands.append(_COPY_COMMAND.format(

source = hdr_file.path,

destination = file.path,

))

for src_file in all_proto_src_files:

dir = _PROTO_DIR.format(path = name)

if "options.nanopb." in src_file.basename:

dir = _OPTIONS_DIR.format(path = name)

file = ctx.actions.declare_file(

_FILENAME.format(dirname = dir, filename = src_file.basename),

)

outputs.append(file)

commands.append(_COPY_COMMAND.format(

source = src_file.path,

destination = file.path,

))

outputs.append(ctx.outputs.zip)

commands.append(_ZIP_COMMAND.format(

output_dir = ctx.outputs.zip.dirname,

zip_filename = ctx.outputs.zip.basename,

))

ctx.actions.run_shell(

inputs = inputs,

outputs = outputs,

command = "\n".join(commands),

)

return struct(

transitive_zip_files = depset([ctx.outputs.zip]),

providers = [CcInfo(

compilation_context = compilation_context,

linking_context = linking_context,

)],

)

nanopb_proto_library = rule(

implementation = _nanopb_proto_library_impl,

outputs = {

"zip": "%{name}.zip", #output needed to be included in platformio_library

},

attrs = {

"deps": attr.label_list(

mandatory = True,

providers = [

ProtoInfo,

],

),

"nanopb_libs": attr.label_list(

providers = [CcInfo],

default = [Label("@nanopb//:nanopb_header")],

),

"nanopb_generator": attr.label(

executable = True,

cfg = "host",

default = Label("@nanopb//:nanopb_generator"),

),

"protoc": attr.label(

executable = True,

cfg = "host",

default = Label("@com_google_protobuf//:protoc"),

),

"_cc_toolchain": attr.label(

default = Label("@bazel_tools//tools/cpp:current_cc_toolchain"),

),

},

provides = [

CcInfo,

],

toolchains = ["@bazel_tools//tools/cpp:toolchain_type"],

fragments = ["cpp"],

host_fragments = ["cpp"],

)

Mr-Anyone · 2024-11-12T06:47:56Z

looking good.

formatting_scripts/dictionary/edited_dictionary.txt

src/software/embedded/robot_diagnostics_cli/robot_diagnostics_cli.py

src/software/embedded/robot_diagnostics_cli/embedded_communication.py

itsarune

great work so far with this refactor. I think it's coming together and don't have any big feedback

…into Andrewyx/DiagnosticsCLI

itsarune · 2025-02-15T08:05:11Z

src/software/embedded/constants/py_constants.py

+ESTOP_PATH_1 = "/dev/ttyACM0"
+ESTOP_PATH_2 = "/dev/ttyUSB0"
+ESTOP_PATH_3 = "/dev/ttyUSB1"


you can use udev rules (https://www.reactivated.net/writing_udev_rules.html#basic) to make the estop appear as /dev/estop

itsarune · 2025-02-15T08:10:24Z

src/software/embedded/robot_diagnostics_cli/embedded_communication.py

+        self.receive_robot_status = tbots_cpp.RobotStatusProtoListener(
+            str(getRobotMulticastChannel(
+                self.channel_id)) + "%" + f"{self.embedded_data.get_network_interface()}",
+            ROBOT_STATUS_PORT,
+            self.__receive_robot_status,
+            True,
+        )
+
+        # Multicast Sender
+        self.primitive_set_sender = tbots_cpp.PrimitiveSetProtoUdpSender(
+            str(getRobotMulticastChannel(
+                self.channel_id)) + "%" + f"{self.embedded_data.get_network_interface()}", PRIMITIVE_PORT, True
+        )


fyi the networking interface changed for this on upstream

itsarune · 2025-02-15T08:11:11Z

src/software/embedded/robot_diagnostics_cli/embedded_communication.py

+        self.epoch_timestamp_seconds = robot_status.time_sent.epoch_timestamp_seconds
+        self.battery_voltage = robot_status.power_status.battery_voltage
+        self.primitive_packet_loss_percentage = robot_status.network_status.primitive_packet_loss_percentage
+        self.primitive_executor_step_time_ms = robot_status.thunderloop_status.primitive_executor_step_time_ms


have you considered keeping this state in self.embedded_data so that all the data is in one place?

itsarune · 2025-02-15T08:15:43Z

src/software/embedded/robot_diagnostics_cli/embedded_data.py

+    def __clamp(self, val: float, min_val: float, max_val: float) -> float:
+        """Simple Math Clamp function (Faster than numpy & fewer dependencies)
+        :param val: Value to clamp
+        :param min_val: Minimum (Lower) Bound
+        :param max_val: Maximum (Upper) Bound
+        """
+        return min(max(val, min_val), max_val)
+
+    def get_rotate_primitive(self, velocity: float) -> Primitive:
+        """Prepares and returns the processed direct control primitive given a velocity
+        :param velocity: Angular Velocity to rotate the robot


the helpers in the file get_rotate_primitive and the other get_xxx_primitive functions aren't really data. Maybe putting these helpers in a different file embedded_control.py may be better

itsarune · 2025-02-15T08:19:06Z

src/software/embedded/robot_diagnostics_cli/robot_diagnostics_cli.py

+                    logging.exception(f"Unknown Exception: {e}")
+                    raise Typer.Exit(code=exit_code)
+                finally:
+                    self.embedded_communication.send_primitive_set(Primitive(stop=StopPrimitive()))


with this finally clause, do you even need to send stop primitives in the other cases?

itsarune · 2025-02-15T08:22:40Z

src/software/embedded/robot_diagnostics_cli/robot_diagnostics_cli.py

+        :param velocity: Velocity to rotate the wheel
+        :param duration_seconds: Duration to move
+        """
+        # TODO: Confirm max speed for wheel rotation (it is currently net robot velocity)


resolve TODO or make an issue

itsarune · 2025-02-15T08:22:49Z

src/software/embedded/robot_diagnostics_cli/robot_diagnostics_cli.py

+            description
+        )
+
+    def emote(self):


make github issue

itsarune · 2025-02-15T08:23:07Z

src/software/embedded/thunderloop.cpp

@@ -222,6 +222,7 @@ void Thunderloop::runLoop()
            network_status_.set_ms_since_last_primitive_received(
                getMilliseconds(time_since_last_primitive_received));

+//            LOG(INFO) << "PRIMITIVE INFO: " << primitive_set_.DebugString();


itsarune

left some feedback

Andrewyx added 4 commits October 17, 2024 21:11

init cli

9bc1c3f

progress

1c6cdc6

working shell

970153c

Merge branch 'master' of https://github.com/UBC-Thunderbots/Software …

569a5b9

…into Andrewyx/DiagnosticsCLI

Andrewyx added the Robot Software label Oct 20, 2024

Andrewyx and others added 11 commits October 20, 2024 19:09

Merge branch 'master' of https://github.com/UBC-Thunderbots/Software …

98d12ad

…into Andrewyx/DiagnosticsCLI

changes

d2b823f

reverted

217eefc

Merge branch 'master' of https://github.com/UBC-Thunderbots/Software …

d2c7688

…into Andrewyx/DiagnosticsCLI

add py_toolchain

b60a399

wip -- adding python cross-compile

d233a2a

cleanup cross compile

750529e

Merge branch 'add_python_cross_compile' of github.com:itsarune/Softwa…

bdf8822

…re into Andrewyx/DiagnosticsCLI

changes

1ab01f9

working

bb48317

redis

6f04f29

itsarune reviewed Nov 6, 2024

View reviewed changes

formatting_scripts/dictionary/edited_dictionary.txt Outdated Show resolved Hide resolved

itsarune reviewed Nov 6, 2024

View reviewed changes

src/software/embedded/ansible/BUILD Outdated Show resolved Hide resolved

itsarune reviewed Nov 6, 2024

View reviewed changes

src/software/embedded/robot_diagnostics_cli/robot_diagnostics_cli.py Outdated Show resolved Hide resolved

itsarune reviewed Nov 6, 2024

View reviewed changes

src/software/embedded/robot_diagnostics_cli/robot_diagnostics_cli.py Show resolved Hide resolved

itsarune reviewed Nov 6, 2024

View reviewed changes

Andrewyx added 4 commits November 8, 2024 11:51

tables

1c99c3b

working packaging

ec9d6f2

archiving working

abdc0aa

sender init

9e83511

Mr-Anyone reviewed Nov 12, 2024

View reviewed changes

itsarune reviewed Nov 13, 2024

View reviewed changes

formatting_scripts/dictionary/edited_dictionary.txt Outdated Show resolved Hide resolved