Check memory use

[tannewt]

I suspect this uses a lot of memory because of all variable names are stored unless they start with _, are const and run through mpy-cross. Here is an example: https://github.com/pimoroni/Pimoroni_CircuitPython_LTR559/blob/master/pimoroni_circuitpython_ltr559.py#L97

A crude way to measure memory is:

import gc
print(gc.mem_free())
import pimoroni_circuitpython_ltr559
print(gc.mem_free())

This will print the free memory before and after import.

I'd also suggest doing further mods as PRs to this repo even if you merge right away. It dives me something to comment on then with suggestions.

[Gadgetoid]

Is there an alternative pattern for giving users some idea of the chip API?

Without those variable names there's no mapping between the bits on the register and any specific sample rate, frequency, etc so I couldn't - for example - do:

ltr559.light.gain = LTR559_ALS_GAIN_4X 

Would you encourage users to add these constants into their user-code on a per-use basis? Or something else?

Agreed regarding pull-requesting any further changes. I'll set up branch protection.

[dglaude]

Would it make sense to move from a library to a package with two sepate files? One for the driver and one for the "constant" that could be used to change the default behaviour. That way, most user will not pay the memory price of unused "variable".

@tannewt documented to me how to do that in this pull request: adafruit/Adafruit_CircuitPython_PM25#7

[tannewt]

Is there an alternative pattern for giving users some idea of the chip API?

Without those variable names there's no mapping between the bits on the register and any specific sample rate, frequency, etc so I couldn't - for example - do:

ltr559.light.gain = LTR559_ALS_GAIN_4X 

Would you encourage users to add these constants into their user-code on a per-use basis? Or something else?

Agreed regarding pull-requesting any further changes. I'll set up branch protection.

I don't think we have any great solution yet. Enum-like classes are the most common. Example: https://github.com/adafruit/Adafruit_CircuitPython_LSM6DS/blob/master/adafruit_lsm6ds/__init__.py#L78 This doesn't actually save anything though because it has the long string names still. It does factor out the validation logic though and enables type checking.

I think this would be best handled by an IntEnum data descriptor. So instead of: https://github.com/pimoroni/Pimoroni_CircuitPython_LTR559/blob/master/pimoroni_circuitpython_ltr559.py#L77-L84 and https://github.com/pimoroni/Pimoroni_CircuitPython_LTR559/blob/master/pimoroni_circuitpython_ltr559.py#L197

led_pulse_freq_khz = RWEnumBits(3, _LTR559_REG_PS_LED, 5, (30, 40, 50, 60, 70, 80, 90, 100))
"""LED pulse frequency in khz between 30khz to 100khz in 10khz steps."""

This would validate incoming values and map the values in the tuple to indices to write.

Overall, I think we need to improve our tooling to add library size to the CI. That way we can track it over time and understand our trade-offs.

@siddacious, what do you think?

[Gadgetoid]

Using magic numbers in-library and some variation of @dglaude's suggestion could work nicely- but if it all gets compiled into one .mpy file then that's somewhat academic. Perhaps a constants.py in the examples directory?

I could definitely look into RWEnumBits and hopefully it's soon enough to avoid ruffling any feathers by changing the API.

I'd love to see compiler metrics attached to builds- that would give me a target to optimise to, although I've yet to run GitHub Actions locally due to most of my dev taking place either in WSL or on a Pi- both targets that aren't so docker-compatible.

Cheers all!

[dglaude]

"my" solution is that there would be a folder with two mpy. One is mandatory, the other one contain other "const".
Most user will only import the base, even if they store both mpy on the flash (but that is not an issue).
Should I give this a try?

[dglaude]

I have implemented "my" idea. It is maybe too confused to be a PR. I will try to find a way to show you the result and the code.

Using *.py file rather than *.mpy on a M4 Express:

• OLD: 158048-139920=18128
• NEW: 158176-141888=16288

Basically:

1. Created a folder
2. Moved your library into it and renamed to init.py
3. Commented out all the unused const
4. Created a separate file constant.py with only the unused const

I believe it could be smaller by removing all the const and replacing by explicit value (and a comment to say what that hex value mean).

[dglaude]

Ok, it is in this branch from my fork.
I have put all the needed file (except two adafruit stock library) into the exemple folder of "idea" branche: https://github.com/dglaude/Pimoroni_CircuitPython_LTR559/tree/idea

Maybe just browse the fork and have a look to understand the logic.

[Gadgetoid]

Thanks for exploring this. For some reason I'm not seeing the mentioned changes in your branch.

This seems like the most memory-efficient approach for people just using the library as-is, and also gives people the opportunity to copy the constant values to get even leaner. An approach using the enum type would always incur a memory/code size penalty that can't then ever be optimised away.

I'd be tempted to- as you suggest- remove all the constants and document the default values with a comment.

[dglaude]

I somehow miss #define in Python... but I am not a Pythonista that can justify the lack of macro processing...and I don't thing putting M4 to use is acceptable. ;-)

Did you notice the name change to remove circuitpython?

Also maybe all the constant should be in a class and not with a prefix, this could save space and avoid using the global namespace.

Again, I have a very basic Python level, so it might not make sense...

[siddacious]

looks like this slipped through my radar!

I like the idea of better enum/CV support but imho something as simple as "here's the numbers you can choose" leaves a bit to be desired, namely: non-contiguous enum values and string/human readable/printable versions, like what a __repr__ or __str__ would return.

I've been mulling over the code size vs. utility vs. readability/sense-making balancing act and while I don't have any solid ideas just yet the main thrust of my considerations has been avoiding paying the costs associated with code that never gets invoked. @tannewt suggested using import for that by moving more stuff into modules, and I think something like that would be a good idea though it's a bit heavy for existing libs

[Gadgetoid]

Still happy to accept improvements here if they're deemed necessary but it's been a few years and nobody has come begging for more RAM.

I don't have the bandwidth to make changes (it takes a lot of effort both physically and mentally to shift over to Feather/CircuitPython dev) unless they're critical bugs.