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.

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

Expand Python pre-commit hook to auto-apply suggestions #52

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 9 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
repos:
# Standard hooks
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.3.0
rev: v4.6.0
hooks:
- id: check-added-large-files
- id: check-ast
Expand All @@ -34,9 +34,15 @@ repos:

# Python hooks
- repo: https://github.com/PyCQA/flake8
rev: 6.0.0
rev: 7.1.1
hooks:
- id: flake8
- id: flake8

- repo: https://github.com/psf/black
rev: 24.8.0
hooks:
- id: black
language_version: python3

# CPP hooks
- repo: local
Expand Down
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[tool.black]
line-length = 120
86 changes: 54 additions & 32 deletions scripts/check-message-compatibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

from typing import Optional

TOPIC_LIST_FILE = 'px4_ros2_cpp/include/px4_ros2/components/message_compatibility_check.hpp'
MESSAGES_DEFINE = 'ALL_PX4_ROS2_MESSAGES'
TOPIC_LIST_FILE = "px4_ros2_cpp/include/px4_ros2/components/message_compatibility_check.hpp"
MESSAGES_DEFINE = "ALL_PX4_ROS2_MESSAGES"


def message_fields_str_for_message_hash(topic_type: str, msgs_dir: str) -> str:
Expand All @@ -20,7 +20,7 @@ def message_fields_str_for_message_hash(topic_type: str, msgs_dir: str) -> str:
"""
filename = f"{msgs_dir}/msg/{topic_type}.msg"
try:
with open(filename, 'r') as file:
with open(filename, "r") as file:
text = file.read()
except IOError:
print(f"Failed to open {filename}")
Expand All @@ -29,15 +29,25 @@ def message_fields_str_for_message_hash(topic_type: str, msgs_dir: str) -> str:
fields_str = ""

# Regular expression to match field types from .msg definitions
msg_field_type_regex = re.compile(
r"(?:^|\n)\s*([a-zA-Z0-9_/]+)(\[[^\]]*\])?\s+(\w+)[ \t]*(=)?"
)
msg_field_type_regex = re.compile(r"(?:^|\n)\s*([a-zA-Z0-9_/]+)(\[[^\]]*\])?\s+(\w+)[ \t]*(=)?")

# Set of basic types
basic_types = {
"bool", "byte", "char", "float32", "float64",
"int8", "uint8", "int16", "uint16", "int32",
"uint32", "int64", "uint64", "string", "wstring"
"bool",
"byte",
"char",
"float32",
"float64",
"int8",
"uint8",
"int16",
"uint16",
"int32",
"uint32",
"int64",
"uint64",
"string",
"wstring",
}

# Iterate over all matches in the text
Expand All @@ -50,7 +60,7 @@ def message_fields_str_for_message_hash(topic_type: str, msgs_dir: str) -> str:
fields_str += f"{type_}{array} {field_name}\n"

if type_ not in basic_types:
if '/' not in type_:
if "/" not in type_:
# Recursive call to handle nested types
fields_str += message_fields_str_for_message_hash(type_, msgs_dir)
else:
Expand All @@ -61,7 +71,7 @@ def message_fields_str_for_message_hash(topic_type: str, msgs_dir: str) -> str:

def hash32_fnv1a_const(s: str) -> int:
"""Computes the 32-bit FNV-1a hash of a given string"""
kVal32Const = 0x811c9dc5
kVal32Const = 0x811C9DC5
kPrime32Const = 0x1000193
hash_value = kVal32Const
for c in s:
Expand All @@ -82,8 +92,9 @@ def snake_to_pascal(name: str) -> str:
return f'{name.replace("_", " ").title().replace(" ", "")}'


def extract_message_type_from_file(filename: str, extract_start_after: Optional[str] = None,
extract_end_before: Optional[str] = None) -> list[str]:
def extract_message_type_from_file(
filename: str, extract_start_after: Optional[str] = None, extract_end_before: Optional[str] = None
) -> list[str]:
"""Extract message type names from a given file"""
with open(filename) as file:
if extract_start_after is not None:
Expand All @@ -109,12 +120,12 @@ def extract_message_type_from_file(filename: str, extract_start_after: Optional[


def compare_files(file1: str, file2: str):
"""Compare two files and print their differences. """
with open(file1, 'r') as f1, open(file2, 'r') as f2:
"""Compare two files and print their differences."""
with open(file1, "r") as f1, open(file2, "r") as f2:
diff = list(difflib.unified_diff(f1.readlines(), f2.readlines(), fromfile=file1, tofile=file2))
if diff:
print(f"Mismatch found between {file1} and {file2}:")
print(''.join(diff), end='\n\n')
print("".join(diff), end="\n\n")
return False
return True

Expand All @@ -125,14 +136,14 @@ def main(repo1: str, repo2: str, verbose: bool = False):
sys.exit(1)

# Retrieve list of message types to check
messages_types = sorted(extract_message_type_from_file(
os.path.join(os.path.dirname(__file__), '..', TOPIC_LIST_FILE),
MESSAGES_DEFINE,
r'^\s*$')
messages_types = sorted(
extract_message_type_from_file(
os.path.join(os.path.dirname(__file__), "..", TOPIC_LIST_FILE), MESSAGES_DEFINE, r"^\s*$"
)
)

if verbose:
print("Checking the following message files:", end='\n\n')
print("Checking the following message files:", end="\n\n")
for msg_type in messages_types:
print(f" - {msg_type}.msg")
print()
Expand All @@ -150,25 +161,36 @@ def main(repo1: str, repo2: str, verbose: bool = False):
else:
if verbose:
for msg_type in incompatible_types:
file1 = os.path.join(repo1, 'msg', f'{msg_type}.msg')
file2 = os.path.join(repo2, 'msg', f'{msg_type}.msg')
file1 = os.path.join(repo1, "msg", f"{msg_type}.msg")
file2 = os.path.join(repo2, "msg", f"{msg_type}.msg")
compare_files(file1, file2)
print("Note: The printed diff includes all content differences. "
"The computed check is less sensitive to formatting and comments.", end='\n\n')
print(
"Note: The printed diff includes all content differences. "
"The computed check is less sensitive to formatting and comments.",
end="\n\n",
)
print("FAILED! Some files differ:")
for msg_type in incompatible_types:
print(f" - {msg_type}.msg")
sys.exit(1)


if __name__ == "__main__":
parser = argparse.ArgumentParser(description="Check message compatibility between two repositories \
using the set of checked messages ALL_PX4_ROS2_MESSAGES.")
parser.add_argument('repo1', help="path to the first repo containing a msg/ directory \
(e.g /path/to/px4_msgs/)")
parser.add_argument('repo2', help="path to the second repo containing a msg/ directory \
(e.g /path/to/PX4-Autopilot/)")
parser.add_argument('-v', '--verbose', dest='verbose', action='store_true', help='verbose output')
parser = argparse.ArgumentParser(
description="Check message compatibility between two repositories \
using the set of checked messages ALL_PX4_ROS2_MESSAGES."
)
parser.add_argument(
"repo1",
help="path to the first repo containing a msg/ directory \
(e.g /path/to/px4_msgs/)",
)
parser.add_argument(
"repo2",
help="path to the second repo containing a msg/ directory \
(e.g /path/to/PX4-Autopilot/)",
)
parser.add_argument("-v", "--verbose", dest="verbose", action="store_true", help="verbose output")
args = parser.parse_args()

main(args.repo1, args.repo2, args.verbose)
48 changes: 27 additions & 21 deletions scripts/check-used-topics.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,23 @@

from typing import Optional

ignored_topics = ['message_format_request', 'message_format_response']
ignored_topics = ["message_format_request", "message_format_response"]

configs = [
# Tuples of (topics_list_file, topic define, source_dir list)
('px4_ros2_cpp/include/px4_ros2/components/message_compatibility_check.hpp', 'ALL_PX4_ROS2_MESSAGES', [
'px4_ros2_cpp/src'
]),
(
"px4_ros2_cpp/include/px4_ros2/components/message_compatibility_check.hpp",
"ALL_PX4_ROS2_MESSAGES",
["px4_ros2_cpp/src"],
),
]

project_root_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), '..')
project_root_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), "..")


def extract_topics_from_file(filename: str, extract_start_after: Optional[str] = None,
extract_end_before: Optional[str] = None) -> list[str]:
def extract_topics_from_file(
filename: str, extract_start_after: Optional[str] = None, extract_end_before: Optional[str] = None
) -> list[str]:
with open(filename) as file:
if extract_start_after is not None:
for line in file:
Expand All @@ -43,9 +46,9 @@ def check(verbose: bool):
for topics_list_file, define, source_list in configs:
topics_list_file = os.path.join(project_root_dir, topics_list_file)

checked_topics = extract_topics_from_file(topics_list_file, define, r'^\s*$')
checked_topics = extract_topics_from_file(topics_list_file, define, r"^\s*$")
if verbose:
print(f'checked topics: {checked_topics}')
print(f"checked topics: {checked_topics}")
assert len(checked_topics) > 0

all_used_topics = []
Expand All @@ -54,47 +57,50 @@ def check(verbose: bool):
# Iterate recursively
for subdir, dirs, files in os.walk(source_dir):
for file in files:
if file.endswith('.hpp') or file.endswith('.cpp'):
if file.endswith(".hpp") or file.endswith(".cpp"):
file_name = os.path.join(subdir, file)

if os.path.normpath(file_name) == os.path.normpath(topics_list_file):
continue

if verbose:
print(f'extracting from file: {file_name}')
print(f"extracting from file: {file_name}")
all_used_topics.extend(extract_topics_from_file(file_name))

all_used_topics = set(all_used_topics) # remove duplicates
if verbose:
print(f'used topics: {all_used_topics}')
print(f"used topics: {all_used_topics}")

not_found_topics = []
for used_topic in all_used_topics:
if used_topic not in checked_topics and used_topic not in ignored_topics:
not_found_topics.append(used_topic)
if len(not_found_topics) > 0:
raise RuntimeError(f'Topic(s) {not_found_topics} are not in the list of checked topics ({define}).\n'
f'Add the topic to the file {topics_list_file}')
raise RuntimeError(
f"Topic(s) {not_found_topics} are not in the list of checked topics ({define}).\n"
f"Add the topic to the file {topics_list_file}"
)

not_used_topics = []
for checked_topic in checked_topics:
if checked_topic not in all_used_topics and checked_topic not in ignored_topics:
not_used_topics.append(checked_topic)
if len(not_used_topics) > 0:
raise RuntimeError(f'Topic(s) {not_used_topics} are in the list of checked topics ({define}), but not '
f'used in code.\n'
f'Remove the topic from the file {topics_list_file}')
raise RuntimeError(
f"Topic(s) {not_used_topics} are in the list of checked topics ({define}), but not "
f"used in code.\n"
f"Remove the topic from the file {topics_list_file}"
)


def main():
parser = argparse.ArgumentParser(description='Check used PX4 topics in source')
parser = argparse.ArgumentParser(description="Check used PX4 topics in source")

parser.add_argument('-v', '--verbose', dest='verbose', action='store_true',
help='Verbose Output')
parser.add_argument("-v", "--verbose", dest="verbose", action="store_true", help="Verbose Output")

args = parser.parse_args()
check(args.verbose)


if __name__ == '__main__':
if __name__ == "__main__":
main()
Loading
Loading