-
Notifications
You must be signed in to change notification settings - Fork 14
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
Update radio test to work with new repo structure #97
base: main
Are you sure you want to change the base?
Conversation
Reading through this it looks good to me! I don't have the hardware to test this right now, but I can try to get to it on Tuesday when I return to the lab. |
Yeah, this was kind of hacky. Just wanted to get something working. I suspect the team will spend a lot more focused on actually testing these interactions. When we do, working and testing stuff in the repl will get easier. |
|
||
while True: | ||
if device_selection == "A": | ||
time.sleep(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if device_selection == "A":
self.number_of_attempts += 1
if self.number_of_attempts >= 5:
print("Too many attempts. Quitting.")
break
time.sleep(1)
self.device_under_test(self.number_of_attempts)
Wants to make 2 changes:
- Check numbers of attempt, it's doing infinite loop atm if there's no receiver
- In print statement, Attempt starts at 0, this will make it starts at 1
Note to myself, when editing the file, edit the file on board, not the local repo. spent more than 10 minutes wondering why the update wasn't working
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code updated
|
||
self.cubesat.radio1.node = 0xFA | ||
self.cubesat.radio1.destination = 0xFB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to correctly "ping-pong" when we are the device_under_test
we should configure as:
self.cubesat.radio1.node = 0xFB
self.cubesat.radio1.destination = 0xFA
This matches the way the we setup the standard flight code.
cubesat.radio1.tx_power = radio_cfg["tx_power"] | ||
cubesat.radio1.receive_timeout = radio_cfg["receive_timeout"] | ||
cubesat.radio1.enable_crc = False | ||
cubesat.f_fsk = True if fsk_or_lora == "fsk" else False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using the ternary operator here is a little confusing. Also, the fsk_or_lora == "fsk"
condition doesn't match the comment above which says # Either "f" for FSK or "l" for LoRa
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point the radio has already been instantiated (in the Satellite
class [which is built on repl boot] that has been passed into the RadioTest
class. Setting fsk here would only impact the next time the repl rebooted... would probably result in unexpected test behavior. Bringing the work from my radio branch could make this kind of fun.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yeah that does raise an interesting challenge. Makes it more relevant than ever that we try to figure out a way to implement good init
and deinit
routines to allow for reconfiguring the hardware during the same runtime without needing a reboot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I agree that we want that functionality. Using FPrime as a model we would only allocate memory once at application start. We wouldn't touch the memory model after that point. Kind of hard to do in Python but in that world we would prefer reboots into the state that we want rather than risk some kind of memory overrun on the running spacecraft. I can understand how that feature might help a ground station controller though.
If we really want it, here's how we might go about implementing that #143
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm you do make a good point. I think that the radio is a particularly tricky case in that switching between fsk
and LoRa
modes kind of requires tearing down a bunch of settings and building them back up. I think on the flight software side it is perfectly acceptable to just initiate a restart if you need to do a major reconfiguration of something like the radio.
Why don't we just stick to a static default of being in LoRa mode for the radio_test.py
that is in this repo for now.
For runtime mode switching, maybe we could take what you have sketched out in #143 and file that into the new ground_station_software repo? It is currently empty, but I plan to add to it all the code needed to bring up a flight controller board as a ground station controller.
self.number_of_attempts += 1 | ||
if self.number_of_attempts >= 5: | ||
print("Too many attempts. Quitting.") | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking from being the transmitter after a certain number of attempts is not a bad idea, but a few thoughts if we intend to continue with this addition:
- The standard practice for using
radio_test.py
right now is to just turn on thedevice_under_test
function and let it run indefinitely while the test is being run. A test operator can simply interrupt the code when they are done. - If we want to auto end the test after certain number of tests, I would recommend making the test duration a dynamic input rather than just hard coding 5 attempts (quite a low number) as the only setting.
- Dynamic input could be accomplished by just added another
input()
call before the test begins where the user can state how long they want the test to run for (bonus points if it can accept time as an input rather than just the number of tests!
Just to keep the contribution history organized it might make sense to break this out into its own independent PR.
|
Hey @nateinaction I was wondering if we could revisit this and approve / merge the PR so we can get this functionality updated and keep this from getting too stale. I think from my requested changes on the |
@Mikefly123 Ok, this has been added to my short list. Will take a look at this hopefully tonight maybe tomorrow night. |
This PR updates the radio test so that it can be easily imported into the repl. It also includes a readme on how to run it. The TX State team was able to successfully send jokes to running flight control board using this test file.