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

Support period in parameter names for COSMOS 6 #1677

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
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

VERTICAL
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

VERTICAL
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,10 @@ export default {
packet: 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 @@ -153,7 +153,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 @@ -69,11 +69,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
tools-router:
rule: Path(`/tools/{id:.*/.*[.].*}`)
rule: Path(`/tools/{id:.*/.*[.](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)}`)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a hack. We should remove and force tools to route their static files from a known route like /files. We should also handle 404s better.

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
base-router:
rule: Path(`/{id:.*[.].*}`)
rule: Path(`/{id:.*[.](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
Loading