Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correct logger message documentation parsing, add to CI #28438

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+74 −48
Open

Correct logger message documentation parsing, add to CI #28438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6de5eb2
autotest: logger_metadata: flake8 cleanliness
peterbarker Oct 19, 2024
a614ece
.github: add logger metadata generation to CI
peterbarker Oct 20, 2024
37f053d
Tools: add logger metadata generation to CI
peterbarker Oct 20, 2024
69b1cf4
autotest: logger_metadata: exempt iofirmware.cpp from checking
peterbarker Oct 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test_scripts.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ jobs:
python-cleanliness,
astyle-cleanliness,
validate_board_list,
logger_metadata,
]
steps:
# git checkout the PR
Expand Down
52 changes: 29 additions & 23 deletions Tools/autotest/logger_metadata/enum_parse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#!/usr/bin/env python

'''
AP_FLAKE8_CLEAN
'''

from __future__ import print_function

import argparse
Expand All @@ -10,6 +14,7 @@
topdir = os.path.join(os.path.dirname(os.path.realpath(__file__)), '../../../')
topdir = os.path.realpath(topdir)


class EnumDocco(object):

vehicle_map = {
Expand All @@ -35,55 +40,54 @@ def match_enum_line(self, line):
# attempts to extract name, value and comment from line.

# Match: " FRED, // optional comment"
m = re.match("\s*([A-Z0-9_a-z]+)\s*,? *(?://[/>]* *(.*) *)?$", line)
m = re.match(r"\s*([A-Z0-9_a-z]+)\s*,? *(?://[/>]* *(.*) *)?$", line)
if m is not None:
return (m.group(1), None, m.group(2))

# Match: " FRED, /* optional comment */"
m = re.match("\s*([A-Z0-9_a-z]+)\s*,? *(?:/[*] *(.*) *[*]/ *)?$", line)
m = re.match(r"\s*([A-Z0-9_a-z]+)\s*,? *(?:/[*] *(.*) *[*]/ *)?$", line)
if m is not None:
return (m.group(1), None, m.group(2))

# Match: " FRED = 17, // optional comment"
m = re.match("\s*([A-Z0-9_a-z]+)\s*=\s*([-0-9]+)\s*,?(?:\s*//[/<]*\s*(.*) *)?$",
m = re.match(r"\s*([A-Z0-9_a-z]+)\s*=\s*([-0-9]+)\s*,?(?:\s*//[/<]*\s*(.*) *)?$",
line)
if m is not None:
return (m.group(1), m.group(2), m.group(3))

# Match: " FRED = 17, // optional comment"
m = re.match("\s*([A-Z0-9_a-z]+) *= *([-0-9]+) *,?(?: */* *(.*) *)? *[*]/ *$",
m = re.match(r"\s*([A-Z0-9_a-z]+) *= *([-0-9]+) *,?(?: */* *(.*) *)? *[*]/ *$",
line)
if m is not None:
return (m.group(1), m.group(2), m.group(3))

# Match: " FRED = 1U<<0, // optional comment"
m = re.match("\s*([A-Z0-9_a-z]+) *= *[(]?1U? *[<][<] *(\d+)(?:, *// *(.*) *)?",
m = re.match(r"\s*([A-Z0-9_a-z]+) *= *[(]?1U? *[<][<] *(\d+)(?:, *// *(.*) *)?",
line)
if m is not None:
return (m.group(1), 1 << int(m.group(2)), m.group(3))

# Match: " FRED = 0xabc, // optional comment"
m = re.match("\s*([A-Z0-9_a-z]+) *= *(?:0[xX]([0-9A-Fa-f]+))(?:, *// *(.*) *)?",
m = re.match(r"\s*([A-Z0-9_a-z]+) *= *(?:0[xX]([0-9A-Fa-f]+))(?:, *// *(.*) *)?",
line)
if m is not None:
return (m.group(1), int(m.group(2), 16), m.group(3))

'''start discarded matches - lines we understand but can't do anything
with:'''
# Match: " FRED = 17, // optional comment"
m = re.match("\s*([A-Z0-9_a-z]+) *= *(\w+) *,?(?: *// *(.*) *)?$",
m = re.match(r"\s*([A-Z0-9_a-z]+) *= *(\w+) *,?(?: *// *(.*) *)?$",
line)
if m is not None:
return (None, None, None)
# Match: " FRED = FOO(17), // optional comment"
m = re.match("\s*([A-Z0-9_a-z]+) *= *(\w+) *\\( *(\w+) *\\) *,?(?: *// *(.*) *)?$",
m = re.match(r"\s*([A-Z0-9_a-z]+) *= *(\w+) *\\( *(\w+) *\\) *,?(?: *// *(.*) *)?$",
line)
if m is not None:
return (None, None, None)


# Match: " FRED = 1U<<0, // optional comment"
m = re.match("\s*([A-Z0-9_a-z]+) *= *[(]?3U? *[<][<] *(\d+)(?:, *// *(.*) *)?",
# Match: " FRED = 1U<<0, // optional comment"
m = re.match(r"\s*([A-Z0-9_a-z]+) *= *[(]?3U? *[<][<] *(\d+)(?:, *// *(.*) *)?",
line)
if m is not None:
return (m.group(1), 1 << int(m.group(2)), m.group(3))
Expand Down Expand Up @@ -112,21 +116,21 @@ def debug(x):
break
line = line.rstrip()
# print("state=%s line: %s" % (state, line))
if re.match("\s*//.*", line):
if re.match(r"\s*//.*", line):
continue
if state == "outside":
if re.match("class .*;", line) is not None:
# forward-declaration of a class
continue
m = re.match("class *([:\w]+)", line)
m = re.match(r"class *([:\w]+)", line)
if m is not None:
in_class = m.group(1)
continue
m = re.match("namespace *(\w+)", line)
m = re.match(r"namespace *(\w+)", line)
if m is not None:
in_class = m.group(1)
continue
m = re.match(".*enum\s*(class)? *([\w]+)\s*(?::.*_t)? *{(.*)};", line)
m = re.match(r".*enum\s*(class)? *([\w]+)\s*(?::.*_t)? *{(.*)};", line)
if m is not None:
# all one one line! Thanks!
enum_name = m.group(2)
Expand All @@ -142,7 +146,7 @@ def debug(x):
enumerations.append(new_enumeration)
continue

m = re.match(".*enum\s*(class)? *([\w]+)\s*(?::.*_t)? *{", line)
m = re.match(r".*enum\s*(class)? *([\w]+)\s*(?::.*_t)? *{", line)
if m is not None:
enum_name = m.group(2)
debug("%s: %s" % (source_file, enum_name))
Expand All @@ -152,15 +156,15 @@ def debug(x):
skip_enumeration = False
continue
if state == "inside":
if re.match("\s*$", line):
if re.match(r"\s*$", line):
continue
if re.match("#if", line):
if re.match(r"#if", line):
continue
if re.match("#endif", line):
if re.match(r"#endif", line):
continue
if re.match("#else", line):
if re.match(r"#else", line):
continue
if re.match(".*}\s*\w*(\s*=\s*[\w:]+)?;", line):
if re.match(r".*}\s*\w*(\s*=\s*[\w:]+)?;", line):
# potential end of enumeration
if not skip_enumeration:
if enum_name is None:
Expand Down Expand Up @@ -189,7 +193,7 @@ def debug(x):
else:
value = int(value)
last_value = value
# print("entry=%s value=%s comment=%s" % (name, value, comment))
# print("entry=%s value=%s comment=%s" % (name, value, comment))
entries.append(EnumDocco.EnumEntry(name, value, comment))
return enumerations

Expand Down Expand Up @@ -219,6 +223,9 @@ def search_for_files(self, dirs_to_search):
continue
if filepath.endswith("libraries/AP_HAL/utility/getopt_cpp.h"):
continue
# Failed to match ( IOEVENT_PWM = EVENT_MASK(1),)
if filepath.endswith("libraries/AP_IOMCU/iofirmware/iofirmware.cpp"):
continue
self.files.append(filepath)
if len(_next):
self.search_for_files(_next)
Expand Down Expand Up @@ -253,4 +260,3 @@ def run(self):
sys.exit(1)

s.run()
# print("Enumerations: %s" % s.enumerations)
58 changes: 35 additions & 23 deletions Tools/autotest/logger_metadata/parse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#!/usr/bin/env python

'''
AP_FLAKE8_CLEAN
'''

from __future__ import print_function

import argparse
Expand Down Expand Up @@ -33,15 +37,21 @@
# Regular expressions for finding message definitions in structure format
re_start_messagedef = re.compile(r"^\s*{?\s*LOG_[A-Z0-9_]+_[MSGTA]+[A-Z0-9_]*\s*,")
re_deffield = r'[\s\\]*"?([\w\-#?%]+)"?\s*'
re_full_messagedef = re.compile(r'\s*LOG_\w+\s*,\s*\w+\([^)]+\)[\s\\]*,' + f'{re_deffield},{re_deffield},' + r'[\s\\]*"?([\w,]+)"?[\s\\]*,' + f'{re_deffield},{re_deffield}' , re.MULTILINE)
re_full_messagedef = re.compile(r'\s*LOG_\w+\s*,\s*\w+\([^)]+\)[\s\\]*,' +
f'{re_deffield},{re_deffield},' +
r'[\s\\]*"?([\w,]+)"?[\s\\]*,' +
f'{re_deffield},{re_deffield}',
re.MULTILINE)
re_fmt_define = re.compile(r'#define\s+(\w+_FMT)\s+"([\w\-#?%]+)"')
re_units_define = re.compile(r'#define\s+(\w+_UNITS)\s+"([\w\-#?%]+)"')
re_mults_define = re.compile(r'#define\s+(\w+_MULTS)\s+"([\w\-#?%]+)"')

# Regular expressions for finding message definitions in Write calls
re_start_writecall = re.compile(r"\s*[AP:]*logger[\(\)]*.Write[StreamingCrcl]*\(")
re_writefield = r'\s*"([\w\-#?%,]+)"\s*'
re_full_writecall = re.compile(r'\s*[AP:]*logger[\(\)]*.Write[StreamingCrcl]*\(' + f'{re_writefield},{re_writefield},{re_writefield},({re_writefield},{re_writefield})?' , re.MULTILINE)
re_full_writecall = re.compile(r'\s*[AP:]*logger[\(\)]*.Write[StreamingCrcl]*\(' +
f'{re_writefield},{re_writefield},{re_writefield},({re_writefield},{re_writefield})?',
re.MULTILINE)

# Regular expression for extracting unit and multipliers from structure
re_units_mults_struct = re.compile(r"^\s*{\s*'([\w\-#?%!/])',"+r'\s*"?([\w\-#?%./]*)"?\s*}')
Expand All @@ -64,6 +74,7 @@
1e-9: "n" # nano-
}


class LoggerDocco(object):

vehicle_map = {
Expand Down Expand Up @@ -93,7 +104,7 @@ class Docco(object):
def __init__(self, name):
self.name = name
self.url = None
if isinstance(name,list):
if isinstance(name, list):
self.description = [None] * len(name)
else:
self.description = None
Expand All @@ -104,24 +115,24 @@ def __init__(self, name):

def add_name(self, name):
# If self.name/description aren't lists, convert them
if isinstance(self.name,str):
if isinstance(self.name, str):
self.name = [self.name]
self.description = [self.description]
# Replace any existing empty descriptions with empty strings
for i in range(0,len(self.description)):
for i in range(0, len(self.description)):
if self.description[i] is None:
self.description[i] = ""
# Extend the name and description lists
if isinstance(name,list):
if isinstance(name, list):
self.name.extend(name)
self.description.extend([None] * len(name))
else:
self.name.append(name)
self.description.append(None)

def set_description(self, desc):
if isinstance(self.description,list):
for i in range(0,len(self.description)):
if isinstance(self.description, list):
for i in range(0, len(self.description)):
if self.description[i] is None:
self.description[i] = desc
else:
Expand All @@ -147,7 +158,7 @@ def set_field_bits(self, field, bits):
count = 0
entries = []
for bit in bits:
entries.append(EnumDocco.EnumEntry(bit, 1<<count, None))
entries.append(EnumDocco.EnumEntry(bit, 1 << count, None))
count += 1
bitmask_name = self.name + field
self.bits_enums.append(EnumDocco.Enumeration(bitmask_name, entries))
Expand All @@ -167,29 +178,29 @@ def set_vehicles(self, vehicles):

def set_fmts(self, fmts):
# If no fields are defined, do nothing
if len(self.fields_order)==0:
if len(self.fields_order) == 0:
return
# Make sure lengths match up
if len(fmts) != len(self.fields_order):
print(f"Number of fmts don't match fields: msg={self.name} fmts={fmts} num_fields={len(self.fields_order)}")
return
# Loop through the list
for idx in range(0,len(fmts)):
for idx in range(0, len(fmts)):
if fmts[idx] in log_fmt_lookup:
self.fields[self.fields_order[idx]]["fmt"] = log_fmt_lookup[fmts[idx]]
else:
print(f"Unrecognised format character: {fmts[idx]} in message {self.name}")

def set_units(self, units, mults):
# If no fields are defined, do nothing
if len(self.fields_order)==0:
if len(self.fields_order) == 0:
return
# Make sure lengths match up
if len(units) != len(self.fields_order) or len(units) != len(mults):
print(f"Number of units/mults/fields don't match: msg={self.name} units={units} mults={mults} num_fields={len(self.fields_order)}")
print(f"Number of units/mults/fields don't match: msg={self.name} units={units} mults={mults} num_fields={len(self.fields_order)}") # noqa:E501
return
# Loop through the list
for idx in range(0,len(units)):
for idx in range(0, len(units)):
# Get the index into fields from field_order
f = self.fields_order[idx]
# Convert unit char to base unit
Expand Down Expand Up @@ -275,7 +286,7 @@ def search_for_files(self, dirs_to_search):
if len(_next):
self.search_for_files(_next)

def parse_messagedef(self,messagedef):
def parse_messagedef(self, messagedef):
# Merge concatinated strings and remove comments
messagedef = re.sub(r'"\s+"', '', messagedef)
messagedef = re.sub(r'//[^\n]*', '', messagedef)
Expand All @@ -299,9 +310,9 @@ def parse_messagedef(self,messagedef):
self.msg_mults_list[d.group(1)] = d.group(5)
return
# Didn't parse
#print(f"Unable to parse: {messagedef}")
# print(f"Unable to parse: {messagedef}")

def search_messagedef_start(self,line,prevmessagedef=""):
def search_messagedef_start(self, line, prevmessagedef=""):
# Look for the start of a structure definition
d = re_start_messagedef.search(line)
if d is not None:
Expand All @@ -325,13 +336,14 @@ def search_messagedef_start(self,line,prevmessagedef=""):

def parse_file(self, filepath):
with open(filepath) as f:
# print("Opened (%s)" % filepath)
# print("Opened (%s)" % filepath)
lines = f.readlines()
f.close()

def debug(x):
pass
# if filepath == "/home/pbarker/rc/ardupilot/libraries/AP_HAL/AnalogIn.h":
# debug = print
# if filepath == "/home/pbarker/rc/ardupilot/libraries/AP_HAL/AnalogIn.h":
# debug = print
state_outside = "outside"
state_inside = "inside"
messagedef = ""
Expand All @@ -346,7 +358,7 @@ def debug(x):
messagedef = ""
if state == state_outside:
# Check for start of a message definition
messagedef = self.search_messagedef_start(line,messagedef)
messagedef = self.search_messagedef_start(line, messagedef)

# Check for fmt/unit/mult #define
u = re_fmt_define.search(line)
Expand Down Expand Up @@ -425,7 +437,7 @@ def emit_output(self):
new_doccos = []
for docco in self.doccos:
if isinstance(docco.name, list):
for name,desc in zip(docco.name, docco.description):
for name, desc in zip(docco.name, docco.description):
tmpdocco = copy.copy(docco)
tmpdocco.name = name
tmpdocco.description = desc
Expand Down Expand Up @@ -463,7 +475,7 @@ def emit_output(self):
mults = self.msg_mults_list[docco.name]
# Apply the units/mults to the docco
if units is not None and mults is not None:
docco.set_units(units,mults)
docco.set_units(units, mults)
elif units is not None or mults is not None:
print(f"Cannot find matching units/mults for message {docco.name}")

Expand Down
11 changes: 9 additions & 2 deletions Tools/scripts/build_ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function install_mavproxy() {
popd
mavproxy_installed=1
# now uninstall the version of pymavlink pulled in by MAVProxy deps:
python -m pip uninstall -y pymavlink
python3 -m pip uninstall -y pymavlink
fi
}

Expand Down Expand Up @@ -512,7 +512,14 @@ for t in $CI_BUILD_TARGET; do

if [ "$t" == "param_parse" ]; then
for v in Rover AntennaTracker ArduCopter ArduPlane ArduSub Blimp AP_Periph; do
python Tools/autotest/param_metadata/param_parse.py --vehicle $v
python3 Tools/autotest/param_metadata/param_parse.py --vehicle $v
done
continue
fi

if [ "$t" == "logger_metadata" ]; then
for v in Rover Tracker Copter Plane Sub Blimp; do
python3 Tools/autotest/logger_metadata/parse.py --vehicle $v
done
continue
fi
Expand Down
Loading