Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the inline creation of the FSK flag in radio_manager to a standalone use_fsk variable, enabling runtime modification of the flag.
- Extracts the
Flag(index=..., bit_index=7)instantiation into a local variableuse_fsk - Updates the
RFM9xManagerconstructor call to use the newuse_fskvariable instead of an inline instantiation
Comments suppressed due to low confidence (2)
main.py:77
- [nitpick] The variable name
use_fskcould be misleading since it holds a Flag object, not a boolean. Consider renaming it tofsk_flagorflag_use_fskfor clarity.
use_fsk = Flag(index=register.FLAG, bit_index=7)
main.py:77
- Add unit tests to verify that modifying the
use_fskflag at runtime correctly updates the radio behavior, ensuring this refactor is covered by automated tests.
use_fsk = Flag(index=register.FLAG, bit_index=7)
hrfarmer
left a comment
There was a problem hiding this comment.
So sorry I missed this pr, but yeah I was planning to have a PR up that would pull the radio modulation from config and get rid of the FSK flag altogether.
|
Hey @hrfarmer, gotcha! Do you have a feel for how long you think it'll take to get these changes in? I do agree that it is probably a much better idea to just pull which modulation you want to use from I can probably open a separate ticket for this, but the reason I wanted to make this change in the meantime was because it doesn't seem like the In any case, because there isn't a way to conveniently edit the NVM flag at the moment it is pretty easy for one radio to accidentally be set to LoRa mode and another to be set to FSK mode without a simple way to swap between the two. |
|
@Mikefly123 I should be able to get it up by monday at latest, I've been pretty slammed by my job recently but I'm trying to get back into the swing of things now |
|
Closing this now, as it will be taken care of by proveskit/pysquared#254 |
Summary
This is a really simple tweak. Currently in the
radio_managerinitialization we are passing an in place creation of anvm Flagobject rather than creating theFlagobject first and then passing it in.This PR just extracts that object creation so you can actually call it in the runtime rather than just have it be a static object that cannot be changed. If this conflicts with the new
modify_configchanges that are incoming for the radio feel free to let me know and reject this change!How was this tested