-
Notifications
You must be signed in to change notification settings - Fork 26
implement a few size reduction changes #33
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
Conversation
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.
@jerryneedell asking some questions, -140 bytes of space saving is 💯
@@ -300,8 +297,26 @@ def __init__( | |||
self.preamble_length = preamble_length # Set the preamble length. | |||
self.frequency_mhz = frequency # Set frequency. | |||
self.encryption_key = encryption_key # Set encryption key. | |||
# set radio configuration parameters | |||
self._configure_radio() |
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.
What's the reason you removed configure_radio
?
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.
putting the code inline rather than as a function call saved 80 bytes!
adafruit_rfm69.py
Outdated
# by default. Users with advanced knowledge can manually reconfigure | ||
# for any other mode (consulting the datasheet is absolutely | ||
# necessary!). | ||
self.data_mode = 0b00 # Packet mode |
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 removed a few register initializations that were setting the registers to their default values.
This got me thinking - are any of these values default?
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.
data_mode and modulation_type are the defaults -- I'll remove them
saved 12 bytes!
self.send( | ||
data, | ||
b"!", |
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.
Why is b"!",
being sent instead of the data
?
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.
just seemed simpler to do it this way. The ACK packet is always just "!"
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.
Note that the original bytes("!", "UTF-8") stores two strings plus the byte codes to convert them to a bytes object.
data = b"!"will save those two strings and only cost the assign byte code assuming the name
data` is used elsewhere.
@brentru hold off on merging this.... I will need to do a lot more testing to resolve this. The issue only occurs when I have an M0 receiving (does not seem to matter which kind) and I am sending "reliable datagrams" when the internal sequence counter is between 128 and 255 (0x80-0xff) the receiver frequently is missing the MSB of the internal sequence count (identifier) ... This results in an ACK failure. I have no idea what is going wrong yet. I have not seen the issue on an RPi or an M4. For these test, I have been using an RPi as one side of the test. I'll keep working on it .... edited to add -- it is not a new problem ... it appears to occur in a similar but even more subtle way with the current release. |
Ok - holding off until that issue is cleared. |
Still investigating the SPI baud rate issue, but wanted to get some of these updated in place. Refactored some code to eliminate a function that was used for writing to the FIFO. now done via the general write_from(). |
@brentru after the discussion at yesterdays CP weekly, I think this is ready to go. There are some long term changes that may be worth looking into but I don't think we have to wait. Let me know if you have any questions about this or if you want any additional changes. |
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.
LGTM!
Updating https://github.com/adafruit/Adafruit_CircuitPython_NeoPixel_SPI to 0.9.0 from 0.8.1: > Merge pull request adafruit/Adafruit_CircuitPython_NeoPixel_SPI#22 from caternuson/tunable_kawrgs Updating https://github.com/adafruit/Adafruit_CircuitPython_RFM69 to 2.1.0 from 2.0.1: > Merge pull request adafruit/Adafruit_CircuitPython_RFM69#33 from jerryneedell/jerryn_size
changes to reduce the code size.
disabled pylint check for "too-many"statements" for the init() function.
without this I had to create a function to do some of the initializations. THis added 80 bytes to the code size.
The init() function is only a few statements over the 50 statement limit in pylint so it seemed better to override.
The savings for these changes is only about 140 bytes total but seems like it is worth doing.
I removed a few register initializations that were setting the registers to their default values.
Tested on RaspberryPi 4 with rfm69 bonnet and feather_m4_express with rfm69 featherwing.