-
Notifications
You must be signed in to change notification settings - Fork 18
Rtc conflict fix #500
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
base: main
Are you sure you want to change the base?
Rtc conflict fix #500
Changes from all commits
75cfa31
f385b3a
94a06e5
bfc9356
51904cf
4d14cf5
435ce42
9302e89
a10dd96
9a031e7
4e04d63
d6000b1
12478c0
7aaf430
0d02d0b
3cd67a9
746e24f
16468ad
6803ab7
6c9a181
6243eaa
ae116fc
fe0dc47
02a605a
081d2b0
0ac06b0
43b16df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -288,6 +288,12 @@ def test_04_sequence_cancellation_on_time_set( | |||||||||||||||
| def test_05_rtc_alarm_set_and_trigger(fprime_test_api: IntegrationTestAPI, start_gds): | ||||||||||||||||
| """Test that we can set an RTC alarm and that it triggers at the correct time""" | ||||||||||||||||
|
|
||||||||||||||||
| # Ensure that we are not using proc time | ||||||||||||||||
| proves_send_and_assert_command( | ||||||||||||||||
| fprime_test_api, f"{rtcManager}.TIMEBASE_PRM_SET", ["TB_SC_TIME"] | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.TimeBaseChanged", timeout=10) | ||||||||||||||||
|
|
||||||||||||||||
| # Clear histories | ||||||||||||||||
| fprime_test_api.clear_histories() | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -302,17 +308,22 @@ def test_05_rtc_alarm_set_and_trigger(fprime_test_api: IntegrationTestAPI, start | |||||||||||||||
| Second=alarm_time.second, | ||||||||||||||||
| ) | ||||||||||||||||
| alarm_time_data_str = json.dumps(alarm_time_data) | ||||||||||||||||
| start: TimeType = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+311
to
+313
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 2 'set_datetime\(' PROVESFlightControllerReference/test/int
rg -n -C 2 'datetime\.now\(timezone\.utc\)' PROVESFlightControllerReference/test/intRepository: Open-Source-Space-Foundation/proves-core-reference Length of output: 15158 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== rtc_test.py imports and nearby helper =="
sed -n '1,80p;255,515p' PROVESFlightControllerReference/test/int/rtc_test.py
echo
echo "== locate TimeType definition/usages =="
rg -n "class TimeType|def set_datetime|TimeBase|TB_DONT_CARE|TB_UTC|TB_" .Repository: Open-Source-Space-Foundation/proves-core-reference Length of output: 24805 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== TimeType source in installed package =="
python3 - <<'PY'
import inspect
from fprime_gds.common.models.serialize.time_type import TimeType
print("module:", TimeType.__module__)
print(inspect.getsourcefile(TimeType))
print("\nset_datetime source:")
try:
print(inspect.getsource(TimeType.set_datetime))
except Exception as e:
print(type(e).__name__, e)
print("\nTimeBase source:")
try:
print(inspect.getsource(TimeType.TimeBase))
except Exception as e:
print(type(e).__name__, e)
print("\nTimeBase values:")
print(list(TimeType.TimeBase))
for val in TimeType.TimeBase:
print(val, val.value, val.name)
PY
echo
echo "== related await_event predicate behavior =="
rg -n -C 3 'def await_event|time_base|TimeBase|TB_' fprime-gds-common-proves-core-reference fprime_gds 2>/dev/null || trueRepository: Open-Source-Space-Foundation/proves-core-reference Length of output: 354 🌐 Web query:
💡 Result: In the F' (F Prime) architecture, TimeType is a serializable class located in fprime_gds.common.models.serialize.time_type used to parse, store, and format timestamp information for telemetry and events [1][2]. Regarding your query on TimeType and time base values: 1. TimeType and set_datetime: The TimeType class provides methods to manage timestamp data [1]. While standard Python datetime conversion is often handled through getters like get_datetime [1], the internal values (seconds and microseconds) are typically managed during object initialization or via internal helpers [1]. Users primarily interact with TimeType through arithmetic operations and comparison operators supported by the GDS Integration Test API [2]. 2. TimeBase Enumerations (TB_DONT_CARE, TB_UTC): The TimeBase enumeration defines the source or clock system for a timestamp [3][4]. - TB_DONT_CARE: Defined as 0xFFFF (or 65535), this value is used primarily in sequences to indicate that the sequence should execute regardless of the current flight software time base [3][4][5]. - TB_UTC: Note that standard F' TimeBase configurations typically include values like TB_NONE, TB_PROC_TIME, TB_WORKSTATION_TIME, and TB_SC_TIME [4][6][5]. While specific project implementations may extend or map time bases differently, TB_UTC is not a standard, built-in member of the default TimeBase enumeration in the core F' framework [4][5]. If you are looking to set a specific time base, you would typically provide the desired TimeBase enum value during the construction of a TimeType object [1]. The system uses these bases to ensure synchronization between ground-side sequences and flight software state [3]. Citations:
🌐 Web query:
💡 Result: In the NASA F Prime (F´) Ground Data System (GDS), the TimeType class is used to store and manage timestamps for telemetry and events [1][2]. While you referenced "TB_UTC," the F Prime GDS implementation typically uses defined TimeBase constants to categorize time sources [1]. The Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== inspect fprime-gds package/source if present =="
python3 - <<'PY'
import shutil, urllib.request, sys
urls = [
"https://raw.githubusercontent.com/nasa/fprime-gds/devel/src/fprime_gds/common/models/serialize/time_type.py",
]
for url in urls:
try:
data = urllib.request.urlopen(url, timeout=20).read().decode("utf-8", "replace")
except Exception as e:
print(f"FETCH_FAILED {url}: {type(e).__name__}: {e}")
continue
lines = data.splitlines()
print(f"--- {url} ({len(lines)} lines) ---")
for i, line in enumerate(lines, 1):
if "class TimeBase" in line or "TB_UTC" in line or "TB_DONT_CARE" in line or "TB_SC_TIME" in line or "TB_PROC_TIME" in line or "def set_datetime" in line:
start=max(1,i-8); end=min(len(lines),i+16)
for j in range(start,end+1):
print(f"{j:4}: {lines[j-1]}")
print()
PY
echo
echo "== list installed fprime-gds location without importing =="
python3 - <<'PY'
import sys, os, zipimport
print("sys.path:")
for p in sys.path[:10]:
print(" ", p)
for spec in pkg_resources_pkgutil := []:
pass
try:
import pkg_resources
for p in pkg_resources.working_set.by_key["fprime-gds"].metadata.resource_listdir("fprime_gds/common/models/serialize") if "fprime-gds" in {k.lower(): True for k in []} and "pkg_resources" in dir() else []:
if "time_type" in p:
print(p)
except Exception as e:
print("pkg_resources check skipped:", type(e).__name__, e)
try:
import fprime_gds
print("fprime_gds imported from", fprime_gds.__file__)
except Exception as e:
print("fprime_gds import failed:", type(e).__name__, e)
PYRepository: Open-Source-Space-Foundation/proves-core-reference Length of output: 704 🌐 Web query:
💡 Result: In the F Prime (F´) framework, TB_DONT_CARE is a member of the TimeBase enumeration, defined with a value of 0xFFFF [1][2]. It is used primarily in sequencing to indicate that a sequence can be executed regardless of the current flight software time base [1]. Regarding the context of your query: 1. TimeType and Test API: The TimeType class is a serializable object used for storing and comparing timestamps (seconds and microseconds) within F Prime [3][4]. While TimeType handles time data, the TB_DONT_CARE constant belongs specifically to the TimeBase enumeration used by the system to manage time synchronization and sequencing [1][2]. 2. Functionality: When a sequence specifies TB_DONT_CARE as its time base, the system bypasses time base validation checks that would otherwise prevent the sequence from running if the flight software time base did not match the expected source [1][5]. 3. Configuration: The constant is defined in the F Prime configuration (typically in FpConfig.h) alongside other time base types like TB_PROC_TIME and TB_WORKSTATION_TIME [2][6][7]. It is distinct from FW_CONTEXT_DONT_CARE, which serves a similar "don't care" purpose for time contexts in sequences [5][8][6]. There is no standard method named set_datetime that takes TB_DONT_CARE as an argument in the public IntegrationTestAPI or TimeType class [3][9]. Time management in the Test API is generally handled through TimeType instances or helper methods like get_latest_time [4][9]. If you are encountering a set_datetime method, it may be a custom implementation or specific to a localized test script [3]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== fetch source with insecure SSL (remote docs/source unavailable to command, use deterministic local evidence instead) =="
curl -kL -sS --max-time 20 "https://raw.githubusercontent.com/nasa/fprime-gds/devel/src/fprime_gds/common/models/serialize/time_type.py" | sed -n '1,180p' || true
echo
echo "== repository ruff/python lint config for timezone warnings =="
rg -n "DTZ005|use-datetime-default|datetime-default|flake8-datetimez|ruff|lint" -g 'pyproject.toml' -g 'Makefile' -g '*.ini' . || trueRepository: Open-Source-Space-Foundation/proves-core-reference Length of output: 6419 🌐 Web query:
💡 Result: In the F Prime (F´) GDS, the TimeType class (found in fprime_gds.common.models.serialize.time_type) is used to handle timestamping data [1]. There is no method named set_datetime in the TimeType class [1]. The TimeType object is initialized using the constructor init(time_base=0, time_context=0, seconds=0, useconds=0) [1]. While the class includes a method called get_datetime(tz=None) to convert a TimeType object into a Python datetime object [1], it does not have a corresponding setter to update the TimeType instance from a datetime object. Regarding TimeBase and TB_UTC: 1. TimeBase: This is an enumeration defining the clocks in the F´ system (e.g., TB_NONE, TB_PROC_TIME, TB_WORKSTATION_TIME, TB_SC_TIME, TB_DONT_CARE) [2][3]. 2. TB_UTC: This is generally not a standard TimeBase enum value in the base F´ framework [2][3]. Time bases are typically defined by the project in FpConfig.fpp [3]. For time manipulation within the GDS test API, the TimeType class supports basic arithmetic (addition, subtraction) and comparisons directly with other TimeType objects or floating-point numbers [4]. If you need to construct a TimeType object, you generally provide the seconds and microseconds directly during instantiation [1]. [1][4][2] Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== locate TimeType definitions across repo =="
rg -n "class TimeType|def set_datetime|TB_UTC|TB_DONT_CARE" . --glob '!**/.git/**' || true
echo
echo "== all TimeType constructors/assignments =="
rg -n "TimeType\(|\.set_datetime\(" . --glob '!**/.git/**' || true
echo
echo "== check whether TB_UTC is defined in git-tracked files =="
rg -n "TB_UTC" $(git ls-files) 2>/dev/null || true
echo
echo "== inspect repository pyproject for DTZ005, if present =="
python3 - <<'PY'
from pathlib import Path
for p in [Path("pyproject.toml"), Path("ruff.toml"), Path(".ruff.toml")]:
if p.exists():
print(f"--- {p} ---")
txt = p.read_text()
for i, line in enumerate(txt.splitlines(), 1):
if "DTZ005" in line or "use-datetime-default" in line or "timedeltas-only-conversion" in line or "datetime-default" in line:
print(f"{i}: {line}")
PYRepository: Open-Source-Space-Foundation/proves-core-reference Length of output: 5142 🌐 Web query:
💡 Result: In the F Prime (F´) framework, TB_UTC is not a standard member of the fprime_gds.common.models.serialize.time_type.TimeType class or the core Fw::Time system. The F Prime time system primarily utilizes time bases such as TB_NONE, TB_WORKSTATION_TIME, and TB_PROC_TIME [1][2]. The confusion regarding TB_UTC often arises from its presence in other unrelated software projects, such as the libzypp library, which uses an enumeration containing TB_UTC for date and time formatting [3][4][5]. Within the F Prime GDS Python codebase, time is managed using the TimeType class [6]. While developers might occasionally define custom time bases or work with external time formats, TB_UTC is not a built-in F Prime constant. If you encounter TB_UTC in an F Prime context, it is likely a custom extension or an error resulting from a misidentification of external library constants. For standard F Prime operations, time bases are typically set using the defined enumerations in the Fw::Time component, and the GDS handles these via the serialization logic in fprime_gds.common.models.serialize.time_type [6][2]. Citations:
Use timezone-aware UTC timestamps for event windows. These test timestamps are used after UTC-based RTC alarm setup, but 🧰 Tools🪛 Ruff (0.16.1)[warning] 312-312: (DTZ005) 🤖 Prompt for AI AgentsSources: Coding guidelines, Linters/SAST tools |
||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_SET", [alarm_time_data_str]) | ||||||||||||||||
|
|
||||||||||||||||
| # Assert that we receive an AlarmTriggered event within 10 seconds | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.AlarmTriggered", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event( | ||||||||||||||||
| f"{rtcManager}.AlarmTriggered", start=start, timeout=10 | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you using assert event everywhere instead of await event? await allows us to be more flexible with waiting for commands to come back from my understanding, but is the timing importtant in this case? |
||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| # make sure the alarm is gone | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_LIST") | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.AlarmNotSet", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event(f"{rtcManager}.AlarmNotSet", start=start, timeout=10) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| # cancellation test | ||||||||||||||||
| # Cancellation test | ||||||||||||||||
| @pytest.mark.uart_only( | ||||||||||||||||
| reason="This test sets the RTC time which triggers the #402 / #404 bugs on PROVES Core Reference" | ||||||||||||||||
| ) | ||||||||||||||||
|
|
@@ -323,7 +334,7 @@ def test_06_rtc_alarm_cancellation(fprime_test_api: IntegrationTestAPI, start_gd | |||||||||||||||
| fprime_test_api.clear_histories() | ||||||||||||||||
|
|
||||||||||||||||
| # Set an alarm for 5 seconds in the future | ||||||||||||||||
| alarm_time = datetime.now(timezone.utc) + timedelta(seconds=5) | ||||||||||||||||
| alarm_time = datetime.now(timezone.utc) + timedelta(seconds=60) | ||||||||||||||||
| alarm_time_data = dict( | ||||||||||||||||
| Year=alarm_time.year, | ||||||||||||||||
| Month=alarm_time.month, | ||||||||||||||||
|
|
@@ -333,12 +344,45 @@ def test_06_rtc_alarm_cancellation(fprime_test_api: IntegrationTestAPI, start_gd | |||||||||||||||
| Second=alarm_time.second, | ||||||||||||||||
| ) | ||||||||||||||||
| alarm_time_data_str = json.dumps(alarm_time_data) | ||||||||||||||||
|
|
||||||||||||||||
| start: TimeType = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| # Send ALARM_SET and await AlarmSet to get the concrete alarm ID | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_SET", [alarm_time_data_str]) | ||||||||||||||||
| alarm_set_evt: EventData = fprime_test_api.assert_event( | ||||||||||||||||
| f"{rtcManager}.AlarmSet", start=start, timeout=5 | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| # Cancel the alarm immediately | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_CANCEL") | ||||||||||||||||
| # Extract the alarm id from the AlarmSet event (first arg assumed to be the ID) | ||||||||||||||||
| alarm_id = None | ||||||||||||||||
| if alarm_set_evt and len(alarm_set_evt.args) > 0: | ||||||||||||||||
| alarm_id = alarm_set_evt.args[0].val | ||||||||||||||||
|
|
||||||||||||||||
| assert alarm_id is not None, "Failed to obtain alarm id from AlarmSet event" | ||||||||||||||||
|
|
||||||||||||||||
| # Cancel the alarm by ID | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_CANCEL", [alarm_id]) | ||||||||||||||||
|
|
||||||||||||||||
| # Assert AlarmCanceled references the same ID | ||||||||||||||||
| alarm_canceled_evt: EventData = fprime_test_api.assert_event( | ||||||||||||||||
| f"{rtcManager}.AlarmCanceled", start=start, timeout=5 | ||||||||||||||||
| ) | ||||||||||||||||
| assert alarm_canceled_evt.args and alarm_canceled_evt.args[0].val == alarm_id, ( | ||||||||||||||||
| f"AlarmCanceled id {alarm_canceled_evt.args[0].val} did not match expected {alarm_id}" | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+372
to
+374
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Do not index an unchecked event argument list. If Proposed fix- assert alarm_canceled_evt.args and alarm_canceled_evt.args[0].val == alarm_id, (
- f"AlarmCanceled id {alarm_canceled_evt.args[0].val} did not match expected {alarm_id}"
- )
+ assert alarm_canceled_evt.args, "AlarmCanceled did not include an alarm ID"
+ assert alarm_canceled_evt.args[0].val == alarm_id, (
+ f"AlarmCanceled id {alarm_canceled_evt.args[0].val} did not match expected {alarm_id}"
+ )📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.16.1)[warning] 372-374: Assertion should be broken down into multiple parts (PT018) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||
|
|
||||||||||||||||
| # Wait until after the scheduled alarm time to ensure it would have fired if not canceled | ||||||||||||||||
| remaining = (alarm_time - datetime.now(timezone.utc)).total_seconds() | ||||||||||||||||
| if remaining > 0: | ||||||||||||||||
| time.sleep(remaining + 1) | ||||||||||||||||
|
|
||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.AlarmTriggered", timeout=10) | ||||||||||||||||
| # Verify no AlarmTriggered for this alarm id (assert that assert_event times out) | ||||||||||||||||
| with pytest.raises(AssertionError): | ||||||||||||||||
| fprime_test_api.assert_event( | ||||||||||||||||
| f"{rtcManager}.AlarmTriggered", start=start, timeout=3 | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| # validation test | ||||||||||||||||
|
|
@@ -354,8 +398,13 @@ def test_07_rtc_alarm_cancel_no_alarm_set( | |||||||||||||||
| fprime_test_api.clear_histories() | ||||||||||||||||
|
|
||||||||||||||||
| # validate that cancel doesn't work without an alarm being present | ||||||||||||||||
| start: TimeType = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_CANCEL", [0]) | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.AlarmNotCanceled", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event( | ||||||||||||||||
| f"{rtcManager}.AlarmNotCanceled", start=start, timeout=10 | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| # list test | ||||||||||||||||
|
|
@@ -368,8 +417,11 @@ def test_08_rtc_alarm_list(fprime_test_api: IntegrationTestAPI, start_gds): | |||||||||||||||
| # Clear histories | ||||||||||||||||
| fprime_test_api.clear_histories() | ||||||||||||||||
|
|
||||||||||||||||
| start: TimeType = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_LIST") | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.AlarmNotSet", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event(f"{rtcManager}.AlarmNotSet", start=start, timeout=10) | ||||||||||||||||
|
|
||||||||||||||||
| # Set an alarm for 5 seconds in the future | ||||||||||||||||
| alarm_time = datetime.now(timezone.utc) + timedelta(seconds=5) | ||||||||||||||||
|
|
@@ -384,8 +436,11 @@ def test_08_rtc_alarm_list(fprime_test_api: IntegrationTestAPI, start_gds): | |||||||||||||||
| alarm_time_data_str = json.dumps(alarm_time_data) | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_SET", [alarm_time_data_str]) | ||||||||||||||||
|
|
||||||||||||||||
| start = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_LIST") | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.AlarmSet", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event(f"{rtcManager}.AlarmSet", start=start, timeout=10) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| @pytest.mark.uart_only( | ||||||||||||||||
|
|
@@ -404,19 +459,22 @@ def test_09_set_alarm_in_past(fprime_test_api: IntegrationTestAPI, start_gds): | |||||||||||||||
| Second=alarm_time.second, | ||||||||||||||||
| ) | ||||||||||||||||
| alarm_time_data_str = json.dumps(alarm_time_data) | ||||||||||||||||
| start: TimeType = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_SET", [alarm_time_data_str]) | ||||||||||||||||
|
|
||||||||||||||||
| # Assert that we receive an AlarmNotSet event within 10 seconds | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.AlarmNotSet", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event(f"{rtcManager}.AlarmNotSet", start=start, timeout=10) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| @pytest.mark.uart_only( | ||||||||||||||||
| reason="This test sets the RTC time which triggers the #402 / #404 bugs on PROVES Core Reference" | ||||||||||||||||
| ) | ||||||||||||||||
| def test_10_double_set_test(fprime_test_api: IntegrationTestAPI, start_gds): | ||||||||||||||||
| """Ensure that double setting an alarm will result in a rejection from the system""" | ||||||||||||||||
| # Set an alarm for 5 seconds in the future | ||||||||||||||||
| alarm_time = datetime.now(timezone.utc) + timedelta(seconds=5) | ||||||||||||||||
| # Set an alarm for 60 seconds in the future | ||||||||||||||||
| alarm_time = datetime.now(timezone.utc) + timedelta(seconds=60) | ||||||||||||||||
| alarm_time_data = dict( | ||||||||||||||||
| Year=alarm_time.year, | ||||||||||||||||
| Month=alarm_time.month, | ||||||||||||||||
|
|
@@ -426,32 +484,114 @@ def test_10_double_set_test(fprime_test_api: IntegrationTestAPI, start_gds): | |||||||||||||||
| Second=alarm_time.second, | ||||||||||||||||
| ) | ||||||||||||||||
| alarm_time_data_str = json.dumps(alarm_time_data) | ||||||||||||||||
| start: TimeType = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_SET", [alarm_time_data_str]) | ||||||||||||||||
| # Assert that we receive an AlarmSet event within 10 seconds | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.AlarmSet", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event(f"{rtcManager}.AlarmSet", start=start, timeout=10) | ||||||||||||||||
|
|
||||||||||||||||
| # Double set the alarm | ||||||||||||||||
| alarm_time = datetime.now(timezone.utc) + timedelta(seconds=5) | ||||||||||||||||
| alarm_time = datetime.now(timezone.utc) + timedelta(seconds=60) | ||||||||||||||||
| alarm_time_data = dict( | ||||||||||||||||
| Year=alarm_time.year, | ||||||||||||||||
| Month=alarm_time.month, | ||||||||||||||||
| Day=alarm_time.day, | ||||||||||||||||
| Hour=alarm_time.hour, | ||||||||||||||||
| Minute=alarm_time.minute, | ||||||||||||||||
| Second=alarm_time.second, | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+496
to
+503
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value Use a dictionary literal. Ruff C408 flags this redundant 🧰 Tools🪛 ast-grep (0.45.0)[info] 503-503: use jsonify instead of json.dumps for JSON output (use-jsonify) 🪛 Ruff (0.16.1)[warning] 496-503: Unnecessary Rewrite as a literal (C408) 🤖 Prompt for AI AgentsSources: Coding guidelines, Linters/SAST tools |
||||||||||||||||
| alarm_time_data_str = json.dumps(alarm_time_data) | ||||||||||||||||
| start = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_SET", [alarm_time_data_str]) | ||||||||||||||||
| # Assert that we receive an AlarmNotSet event within 10 seconds | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.AlarmNotSet", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event(f"{rtcManager}.AlarmNotSet", start=start, timeout=10) | ||||||||||||||||
|
|
||||||||||||||||
| # Clean up: cancel the alarm | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_CANCEL") | ||||||||||||||||
|
Comment on lines
+512
to
+513
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Pass the required alarm ID during cleanup.
Pass the configured alarm ID ( Proposed fix # Clean up: cancel the alarm
- fprime_test_api.send_command(f"{rtcManager}.ALARM_CANCEL")
+ fprime_test_api.send_command(f"{rtcManager}.ALARM_CANCEL", [0])📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like coderabbit is right about the U16, is this passing without its paramaer?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 4 '\bALARM_CANCEL\b|ALARM_CANCEL_cmdHandler\b' .Length of output: 19125
Line 513 only calls Use 🧬 Code Graph Analysis Results
|
||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| @pytest.mark.uart_only(reason="Test functionality of the timebase parameter") | ||||||||||||||||
| def test_11_proc_toggle(fprime_test_api: IntegrationTestAPI, start_gds): | ||||||||||||||||
| """Test for events emitted by the timebase parameter""" | ||||||||||||||||
|
|
||||||||||||||||
| start: TimeType = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| try: | ||||||||||||||||
| # Test that we can set timebase to proc time | ||||||||||||||||
| proves_send_and_assert_command( | ||||||||||||||||
| fprime_test_api, f"{rtcManager}.TIMEBASE_PRM_SET", ["TB_PROC_TIME"] | ||||||||||||||||
| ) | ||||||||||||||||
| # Assert that we receive a TimeBaseChanged event within 10 seconds | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.TimeBaseChanged", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event( | ||||||||||||||||
| f"{rtcManager}.TimeBaseChanged", start=start, timeout=10 | ||||||||||||||||
| ) | ||||||||||||||||
| finally: | ||||||||||||||||
| # Restore spacecraft time so subsequent tests see RTC-backed timestamps | ||||||||||||||||
| proves_send_and_assert_command( | ||||||||||||||||
| fprime_test_api, f"{rtcManager}.TIMEBASE_PRM_SET", ["TB_SC_TIME"] | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.await_event(f"{rtcManager}.TimeBaseChanged", timeout=10) | ||||||||||||||||
| fprime_test_api.assert_event( | ||||||||||||||||
| f"{rtcManager}.TimeBaseChanged", start=start, timeout=10 | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| @pytest.mark.uart_only(reason="Test functionality of the timebase parameter") | ||||||||||||||||
| def test_12_param_update_conflict_check(fprime_test_api: IntegrationTestAPI, start_gds): | ||||||||||||||||
| """Test for that RTC alarms no longer conflict with parameter updates and function properly""" | ||||||||||||||||
|
|
||||||||||||||||
| start: TimeType = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| # Ensure that we are not using proc time | ||||||||||||||||
| proves_send_and_assert_command( | ||||||||||||||||
| fprime_test_api, f"{rtcManager}.TIMEBASE_PRM_SET", ["TB_SC_TIME"] | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.assert_event( | ||||||||||||||||
| f"{rtcManager}.TimeBaseChanged", start=start, timeout=10 | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| # Set an alarm for 60 seconds in the future | ||||||||||||||||
| alarm_time = datetime.now(timezone.utc) + timedelta(seconds=60) | ||||||||||||||||
| alarm_time_data = dict( | ||||||||||||||||
| Year=alarm_time.year, | ||||||||||||||||
| Month=alarm_time.month, | ||||||||||||||||
| Day=alarm_time.day, | ||||||||||||||||
| Hour=alarm_time.hour, | ||||||||||||||||
| Minute=alarm_time.minute, | ||||||||||||||||
| Second=alarm_time.second, | ||||||||||||||||
| ) | ||||||||||||||||
| alarm_time_data_str = json.dumps(alarm_time_data) | ||||||||||||||||
| start: TimeType = TimeType().set_datetime( | ||||||||||||||||
| datetime.now(), time_base=TimeType.TimeBase("TB_DONT_CARE") | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| # Set an alarm | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_SET", [alarm_time_data_str]) | ||||||||||||||||
| fprime_test_api.assert_event(f"{rtcManager}.AlarmSet", start=start, timeout=10) | ||||||||||||||||
|
Comment on lines
+573
to
+576
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Capture a new event-window timestamp for each command.
🤖 Prompt for AI Agents |
||||||||||||||||
|
|
||||||||||||||||
| # Switch to proc time to make sure it is canceled | ||||||||||||||||
| proves_send_and_assert_command( | ||||||||||||||||
| fprime_test_api, f"{rtcManager}.TIMEBASE_PRM_SET", ["TB_SC_TIME"] | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on the comment should we swap to TB_PROC_TIME instead of TB_SC_TIME? |
||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| # make sure that it is gone | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_LIST") | ||||||||||||||||
| fprime_test_api.assert_event(f"{rtcManager}.AlarmNotSet", start=start, timeout=10) | ||||||||||||||||
|
|
||||||||||||||||
| # Make sure we cannot set while in proc time | ||||||||||||||||
| fprime_test_api.send_command(f"{rtcManager}.ALARM_SET", [alarm_time_data_str]) | ||||||||||||||||
| fprime_test_api.assert_event(f"{rtcManager}.AlarmNotSet", start=start, timeout=10) | ||||||||||||||||
|
|
||||||||||||||||
| # Set time back to RTC to avoid ruining other tests | ||||||||||||||||
| proves_send_and_assert_command( | ||||||||||||||||
| fprime_test_api, f"{rtcManager}.TIMEBASE_PRM_SET", ["TB_SC_TIME"] | ||||||||||||||||
| ) | ||||||||||||||||
| fprime_test_api.assert_event( | ||||||||||||||||
| f"{rtcManager}.TimeBaseChanged", start=start, timeout=10 | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+575
to
+597
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Establish the conflict state before asserting its result. Line 573 does not confirm that Line 577 selects After selecting processor time, assert Proposed fix fprime_test_api.send_command(f"{rtcManager}.ALARM_SET", [alarm_time_data_str])
+ fprime_test_api.assert_event(
+ f"{rtcManager}.AlarmSet", start=start, timeout=10
+ )
- fprime_test_api, f"{rtcManager}.TIMEBASE_PRM_SET", ["TB_SC_TIME"]
+ fprime_test_api, f"{rtcManager}.TIMEBASE_PRM_SET", ["TB_PROC_TIME"]🤖 Prompt for AI Agents |
||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.