Skip to content

Commit

Permalink
Merge pull request #1677 from OpenC3/no_period
Browse files Browse the repository at this point in the history
Support period in parameter names for COSMOS 6
  • Loading branch information
jmthomas authored Nov 15, 2024
2 parents 19c5fef + 80d29b5 commit 792980a
Show file tree
Hide file tree
Showing 15 changed files with 127 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ TELEMETRY <%= target_name %> PARAMS BIG_ENDIAN "Params set by SETPARAMS command"
STATE GOOD 0 GREEN
STATE BAD 1 RED
<% end %>
APPEND_ITEM P_2.2,2 32 UINT "Test weird characters"
APPEND_ITEM P-3+3=3 32 UINT "Test weird characters"
APPEND_ITEM P4!@#$%^&*? 32 UINT "Test weird characters"
APPEND_ITEM P</5|\> 32 UINT "Test weird characters"
APPEND_ITEM P(:6;) 32 UINT "Test weird characters"
ITEM PACKET_TIME 0 0 DERIVED "Ruby time based on TIMESEC and TIMEUS"
READ_CONVERSION unix_time_conversion.rb TIMESEC TIMEUS

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ def initialize(target_name)
packet.value3 = 2
packet.value4 = 1
packet.value5 = 0
packet.write('P_2.2,2', 2)
packet.write('P-3+3=3', 3)
packet.write('P4!@#$%^&*?', 4)
packet.write('P</5|\>', 5)
packet.write('P(:6;)', 6)

packet = @tlm_packets['IMAGE']
packet.enable_method_missing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ SCREEN AUTO AUTO 1.0
VERTICAL
TITLE Params

HORIZONTAL
HORIZONTAL 20
MATRIXBYCOLUMNS 3
LABEL VALUE5
SETTING WIDTH 100 # Only need width on the first row
Expand Down Expand Up @@ -60,7 +60,13 @@ VERTICAL
SETTING LED_COLOR GOOD GREEN
SETTING LED_COLOR BAD RED
END
SETTING RAW margin-left 20px
VERTICAL
LABELVALUE <%= target_name %> PARAMS P_2.2,2
LABELVALUE <%= target_name %> PARAMS P-3+3=3
LABELVALUE <%= target_name %> PARAMS P4!@#$%^&*?
LABELVALUE <%= target_name %> PARAMS P</5|\>
LABELVALUE <%= target_name %> PARAMS P(:6;)
END
END

HORIZONTAL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ TELEMETRY <%= target_name %> PARAMS BIG_ENDIAN "Params set by SETPARAMS command"
STATE GOOD 0 GREEN
STATE BAD 1 RED
<% end %>
APPEND_ITEM P_2.2,2 32 UINT "Test weird characters"
APPEND_ITEM P-3+3=3 32 UINT "Test weird characters"
APPEND_ITEM P4!@#$%^&*? 32 UINT "Test weird characters"
APPEND_ITEM P</5|\> 32 UINT "Test weird characters"
APPEND_ITEM P(:6;) 32 UINT "Test weird characters"
ITEM PACKET_TIME 0 0 DERIVED "Python time based on TIMESEC and TIMEUS"
READ_CONVERSION openc3/conversions/unix_time_conversion.py TIMESEC TIMEUS

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ def __init__(self, target_name):
packet.write("value3", 2)
packet.write("value4", 1)
packet.write("value5", 0)
packet.write('P_2.2,2', 2)
packet.write('P-3+3=3', 3)
packet.write('P4!@#$%^&*?', 4)
packet.write('P</5|\>', 5)
packet.write('P(:6;)', 6)

packet = self.tlm_packets["IMAGE"]
packet.write("CcsdsSeqFlags", "NOGROUP")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ SCREEN AUTO AUTO 1.0
VERTICAL
TITLE Params

HORIZONTAL
HORIZONTAL 20
MATRIXBYCOLUMNS 3
LABEL VALUE5
SETTING WIDTH 100 # Only need width on the first row
Expand Down Expand Up @@ -60,7 +60,13 @@ VERTICAL
SETTING LED_COLOR GOOD GREEN
SETTING LED_COLOR BAD RED
END
SETTING RAW margin-left 20px
VERTICAL
LABELVALUE <%= target_name %> PARAMS P_2.2,2
LABELVALUE <%= target_name %> PARAMS P-3+3=3
LABELVALUE <%= target_name %> PARAMS P4!@#$%^&*?
LABELVALUE <%= target_name %> PARAMS P</5|\>
LABELVALUE <%= target_name %> PARAMS P(:6;)
END
END

HORIZONTAL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,10 @@ export default {
this.rawDialogs.splice(i, 1)
},
openCmdSender(target_name, packet_name) {
window.open(`/tools/cmdsender/${target_name}/${packet_name}`, '_blank')
window.open(
`/tools/cmdsender/${encodeURIComponent(target_name)}/${encodeURIComponent(packet_name)}`,
'_blank',
)
},
currentItems(event) {
this.visible = event.map((i) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,10 @@ export default {
this.rawDialogs.splice(i, 1)
},
openPktViewer(target_name, packet_name) {
window.open(`/tools/packetviewer/${target_name}/${packet_name}`, '_blank')
window.open(
`/tools/packetviewer/${encodeURIComponent(target_name)}/${encodeURIComponent(packet_name)}`,
'_blank',
)
},
currentItems(event) {
this.visible = event.map((i) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ export default {
action: () => {
window.open(
'/tools/tlmgrapher/' +
this.parameters[0] +
encodeURIComponent(this.parameters[0]) +
'/' +
this.parameters[1] +
encodeURIComponent(this.parameters[1]) +
'/' +
this.parameters[2],
encodeURIComponent(this.parameters[2]),
'_blank',
)
},
Expand Down
12 changes: 6 additions & 6 deletions openc3-traefik/traefik.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ http:
service: service-script
priority: 7
# Route to other tool plugins hosted statically in Minio
# Matches any path with a file extension which is assumed to be
# a static file
# Matches any path with a valid file extension which is assumed to be a static file
# TODO: We need to make all static tool files use a fixed route (apply to all traefik.yaml files)
tools-router:
rule: PathRegexp(`/tools/.*/.*[.].*`)
rule: PathRegexp(`/tools/.*/.*[.](ttf|otf|woff|woff2|html|js|css|png|jpg|jpeg|gif|svg|ico|json|xml|txt|pdf|zip|tar|gz|tgz|csv|tsv|md|yaml|yml|bin|doc|docx|xls|xlsx|ppt|pptx|mp4|mp3|wav|avi|mov|flv|swf|apk|ipa|deb|rpm|exe|msi|dmg|pkg|sh|bat|cmd|ps1|py|pl|rb|php|java|class|jar|war|ear|so|dll|lib|a|o|obj|pdb|pdb|lib|dylib|framework)`)
service: service-minio
priority: 6
# Route to other tool plugins hosted statically in Minio
Expand Down Expand Up @@ -81,10 +81,10 @@ http:
service: service-minio
priority: 3
# Route to base files hosted statically in Minio
# Matches any path with a file extension which is assumed to be
# a static file
# Matches any path with a valid file extension which is assumed to be a static file
# TODO: We need to make all static tool files use a fixed route (apply to all traefik.yaml files)
base-router:
rule: PathRegexp(`/.*[.].*`)
rule: PathRegexp(`/.*[.](ttf|otf|woff|woff2|html|js|css|png|jpg|jpeg|gif|svg|ico|json|xml|txt|pdf|zip|tar|gz|tgz|csv|tsv|md|yaml|yml|bin|doc|docx|xls|xlsx|ppt|pptx|mp4|mp3|wav|avi|mov|flv|swf|apk|ipa|deb|rpm|exe|msi|dmg|pkg|sh|bat|cmd|ps1|py|pl|rb|php|java|class|jar|war|ear|so|dll|lib|a|o|obj|pdb|pdb|lib|dylib|framework)`)
middlewares:
# add /tools/base to the beginning
- "addToolsBase"
Expand Down
9 changes: 6 additions & 3 deletions openc3/lib/openc3/config/config_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def verify_num_parameters(min_num_params, max_num_params, usage = "")

# Verifies the indicated parameter in the config doesn't start or end
# with an underscore, doesn't contain a double underscore or double bracket,
# doesn't contain spaces and doesn't start with a close bracket.
# doesn't contain spaces, quotes or brackets.
#
# @param [Integer] index The index of the parameter to check
def verify_parameter_naming(index, usage = "")
Expand All @@ -265,8 +265,11 @@ def verify_parameter_naming(index, usage = "")
if param.include? ' '
raise Error.new(self, "Parameter #{index} (#{param}) for #{@keyword} cannot contain a space (' ').", usage, @url)
end
if param.start_with?('}')
raise Error.new(self, "Parameter #{index} (#{param}) for #{@keyword} cannot start with a close bracket ('}').", usage, @url)
if param.include?('"') or param.include?("'")
raise Error.new(self, "Parameter #{index} (#{param}) for #{@keyword} cannot contain a quote (' or \").", usage, @url)
end
if param.include?('{') or param.include?('}')
raise Error.new(self, "Parameter #{index} (#{param}) for #{@keyword} cannot contain a curly bracket ('{' or '}').", usage, @url)
end
end

Expand Down
14 changes: 11 additions & 3 deletions openc3/python/openc3/config/config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ def verify_num_parameters(self, min_num_params, max_num_params, usage=""):

# Verifies the indicated parameter in the config doesn't start or end
# with an underscore, doesn't contain a double underscore or double bracket,
# doesn't contain spaces and doesn't start with a close bracket.
# doesn't contain spaces, quotes or brackets.
#
# self.param [Integer] index The index of the parameter to check
def verify_parameter_naming(self, index, usage=""):
Expand Down Expand Up @@ -175,10 +175,18 @@ def verify_parameter_naming(self, index, usage=""):
self.url,
)

if param[0] == "}":
if "'" in param or '"' in param:
raise ConfigParser.Error(
self,
f"Parameter {index} ({param}) for {self.keyword} cannot start with a close bracket ('}}').",
f"Parameter {index} ({param}) for {self.keyword} cannot contain a quote (' or \").",
usage,
self.url,
)

if "{" in param or "}" in param:
raise ConfigParser.Error(
self,
f"Parameter {index} ({param}) for {self.keyword} cannot contain a curly bracket ('{{' or '}}').",
usage,
self.url,
)
Expand Down
46 changes: 37 additions & 9 deletions openc3/python/test/config/test_config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ def test_optionally_yields_comment_lines(self):
tf.seek(0)

lines = []
for keyword, params in self.cp.parse_file(tf.name, True):
for _, _ in self.cp.parse_file(tf.name, True):
lines.append(self.cp.line)

self.assertIn("# This is a comment", lines)
Expand Down Expand Up @@ -344,7 +344,7 @@ def test_verifies_the_minimum_number_of_parameters(self):
tf.writelines(line)
tf.seek(0)

for keyword, params in self.cp.parse_file(tf.name):
for keyword, _ in self.cp.parse_file(tf.name):
self.assertEqual(keyword, "KEYWORD")
self.assertRaisesRegex(
ConfigParser.Error,
Expand All @@ -361,7 +361,7 @@ def test_verifies_the_maximum_number_of_parameters(self):
tf.writelines(line)
tf.seek(0)

for keyword, params in self.cp.parse_file(tf.name):
for keyword, _ in self.cp.parse_file(tf.name):
self.assertEqual(keyword, "KEYWORD")
self.assertRaisesRegex(
ConfigParser.Error,
Expand All @@ -372,13 +372,23 @@ def test_verifies_the_maximum_number_of_parameters(self):
)
tf.close()

def test_verifies_parameters_do_not_have_bad_characters(self):
def test_allows_parameters_with_most_characters(self):
tf = tempfile.NamedTemporaryFile(mode="w+t")
line = "KEYWORD BAD1_ BAD__2 'BAD 3' }BAD_4 BAD[[5]]"
line = "KEYWORD P[1] P_2.2,2 P-3+3=3 P4!@#$%^&*? P</5|> P(:6;)"
tf.writelines(line)
tf.seek(0)

for keyword, params in self.cp.parse_file(tf.name):
self.assertEqual(keyword, "KEYWORD")
self.assertListEqual(params, ["P[1]", "P_2.2,2", "P-3+3=3", "P4!@#$%^&*?", "P</5|>", "P(:6;)"])

def test_verifies_parameters_do_not_have_bad_characters(self):
tf = tempfile.NamedTemporaryFile(mode="w+t")
line = "KEYWORD BAD1_ BAD__2 'BAD 3' BAD{4 BAD}4 BAD[[6]] BAD'7 BAD\"8"
tf.writelines(line)
tf.seek(0)

for _, _ in self.cp.parse_file(tf.name):
self.assertRaisesRegex(
ConfigParser.Error,
"cannot end with an underscore",
Expand All @@ -399,16 +409,34 @@ def test_verifies_parameters_do_not_have_bad_characters(self):
)
self.assertRaisesRegex(
ConfigParser.Error,
"cannot start with a close bracket",
"cannot contain a curly bracket",
self.cp.verify_parameter_naming,
4,
)
self.assertRaisesRegex(
ConfigParser.Error,
"cannot contain double brackets",
"cannot contain a curly bracket",
self.cp.verify_parameter_naming,
5,
)
self.assertRaisesRegex(
ConfigParser.Error,
"cannot contain double brackets",
self.cp.verify_parameter_naming,
6,
)
self.assertRaisesRegex(
ConfigParser.Error,
"cannot contain a quote",
self.cp.verify_parameter_naming,
7,
)
self.assertRaisesRegex(
ConfigParser.Error,
"cannot contain a quote",
self.cp.verify_parameter_naming,
8,
)
tf.close()

def test_returns_an_error(self):
Expand All @@ -417,7 +445,7 @@ def test_returns_an_error(self):
tf.writelines(line)
tf.seek(0)

for keyword, params in self.cp.parse_file(tf.name):
for _, _ in self.cp.parse_file(tf.name):
error = self.cp.error("Hello")
self.assertIn("Hello", repr(error))
self.assertEqual(error.keyword, "KEYWORD")
Expand All @@ -432,7 +460,7 @@ def test_collects_all_errors_and_returns_all(self):
tf.seek(0)

try:
for keyword, params in self.cp.parse_file(tf.name):
for keyword, _ in self.cp.parse_file(tf.name):
if keyword == "KEYWORD1":
raise self.cp.error("Invalid KEYWORD1")
# TODO: This doesn't work in Python like Ruby
Expand Down
22 changes: 19 additions & 3 deletions openc3/spec/config/config_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -416,18 +416,34 @@ module OpenC3
end

describe "verify_parameter_naming" do
it "allows most characters in parameters" do
tf = Tempfile.new('unittest')
line = "KEYWORD P[1] P_2.2,2 P-3+3=3 P4!@#$%^&*? P</5|> P(:6;)"
tf.puts line
tf.close

@cp.parse_file(tf.path) do |keyword, params|
expect(keyword).to eql "KEYWORD"
expect(params).to eql ["P[1]", "P_2.2,2", "P-3+3=3", "P4!@#$%^&*?", "P</5|>", "P(:6;)"]
end
tf.unlink
end

it "verifies parameters do not have bad characters" do
tf = Tempfile.new('unittest')
line = "KEYWORD BAD1_ BAD__2 'BAD 3' }BAD_4 BAD[[5]]"
line = "KEYWORD BAD1_ BAD__2 'BAD 3' BAD{4 BAD}4 BAD[[6]] BAD'7 BAD\"8"
tf.puts line
tf.close

@cp.parse_file(tf.path) do |_keyword, _params|
expect { @cp.verify_parameter_naming(1) }.to raise_error(ConfigParser::Error, /cannot end with an underscore/)
expect { @cp.verify_parameter_naming(2) }.to raise_error(ConfigParser::Error, /cannot contain a double underscore/)
expect { @cp.verify_parameter_naming(3) }.to raise_error(ConfigParser::Error, /cannot contain a space/)
expect { @cp.verify_parameter_naming(4) }.to raise_error(ConfigParser::Error, /cannot start with a close bracket/)
expect { @cp.verify_parameter_naming(5) }.to raise_error(ConfigParser::Error, /cannot contain double brackets/)
expect { @cp.verify_parameter_naming(4) }.to raise_error(ConfigParser::Error, /cannot contain a curly bracket/)
expect { @cp.verify_parameter_naming(5) }.to raise_error(ConfigParser::Error, /cannot contain a curly bracket/)
expect { @cp.verify_parameter_naming(6) }.to raise_error(ConfigParser::Error, /cannot contain double brackets/)
expect { @cp.verify_parameter_naming(7) }.to raise_error(ConfigParser::Error, /cannot contain a quote/)
expect { @cp.verify_parameter_naming(8) }.to raise_error(ConfigParser::Error, /cannot contain a quote/)
end
tf.unlink
end
Expand Down
2 changes: 1 addition & 1 deletion playwright/tests/data-extractor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ test('cancels a process', async ({ page, utils }) => {

test('adds an entire target', async ({ page, utils }) => {
await utils.addTargetPacketItem('INST')
await expect(page.getByText('1-20 of 154')).toBeVisible()
await expect(page.getByText('1-20 of 159')).toBeVisible()
})

test('adds an entire packet', async ({ page, utils }) => {
Expand Down

0 comments on commit 792980a

Please sign in to comment.