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

Fix up edl_cmd_shell for os commands #44

Merged
merged 4 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ classifiers = [
]
dependencies = [
"bitstring",
"cfdp-py",
"cfdp-py==0.1.2",
"dataclasses-json",
"oresat-configs",
"oresat-olaf>=3.5.0",
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
bitstring
black
build
cfdp-py
cfdp-py==0.1.2
dataclasses-json
isort
oresat-configs
Expand Down
6 changes: 4 additions & 2 deletions scripts/edl_cmd_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ def help_sdo_write(self):
def do_sdo_write(self, arg: str):
"""Do the sdo_write command."""

args = arg.split(" ")
args = arg.split(" ", maxsplit=3)
if len(args) != 4:
self.help_sdo_write()
return
Expand Down Expand Up @@ -284,8 +284,10 @@ def do_sdo_write(self, arg: str):
value = float(args[3])
elif obj.data_type == canopen.objectdictionary.VISIBLE_STRING:
value = args[3]
elif obj.data_type == canopen.objectdictionary.DOMAIN:
value = args[3].encode("ascii")
Copy link
Member

Choose a reason for hiding this comment

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

This assume the value is string, domain type are void, so this wont work in all cases. It would be more robust to expect the user to past a hex string for domain types.

Copy link
Member

Choose a reason for hiding this comment

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

I guess the quick encode a string will work for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good point, I would prefer to do it properly over quick hacks. Looking through CiA-301 and oresat-configs it feels like while officially undefined the intended use for DOMAIN is for file transfers; if the data is short enough to be written as a hex string easily it should generally be OCTET_STRING. What if instead for DOMAIN we expect a filename and do_sdo_write() opens and sends the contents?

On the other hand it looks like the type is wrong for OS command in oresat-configs. The command should be OCTET_STRING instead of DOMAIN (CiA-301 7.4.8.6). Additionally 7.5.2.26 says command should be "ISO8859 [ASCII] characters or binary". So given that, what if in do_sdo_write() for OCTET_STRING we try to decode the argument from a hex string to bytes and if that fails then just .encode("ascii") the given string? That way we cans still send shell commands directly through sdo_write.

In code:

...
elif obj.data_type == canopen.objectdictionary.DOMAIN:
    with open(arg[3], 'rb') as f:
        value = f.read()
elif obj.data_type == canopen.objectdictionary.OCTET_STRING:
    try:
        value = bytes.fromhex(arg[3])
    except ValueError:
        value = arg[3].encode("ascii")

Copy link
Member

Choose a reason for hiding this comment

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

Yea, that sounds like a good plan

else:
print("invaid")
print(f"invalid OD obj type {obj} 0x{obj.data_type:X}")
return

raw = obj.encode_raw(value)
Expand Down
1 change: 1 addition & 0 deletions scripts/edl_file_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def run(self):
packet = EdlPacket(payload, self._sequence_number, SRC_DEST_ORESAT)
message = packet.pack(self._hmac_key)
uplink.send(message)
print("Current sequence number:", self._sequence_number)
self._sequence_number += 1
time.sleep(self._delay)

Expand Down
Loading