-
Notifications
You must be signed in to change notification settings - Fork 517
Lazy loading test for zigbee switch #2469
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
|
Channel deleted. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against d42421a |
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.
Where's this guy getting used?
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.
ope, this is something sitting in my repo that I used with a different test. It isnt being used, but I will remove it.
|
Also you'll need to add the copyright disclaimer to all the new lua files. |
Is this something that is definitely required for every single file for our license to apply, or is it just a practice that we have been following. I ask because adding these comments at the top of every file negatively affects memory of our driver processes. When a lua file is run it is loaded into memory and parsed into bytecode which is why comments affect the memory of a lua program. That load step brings in the comment data, which even though is not included in bytecode, it does affect the processes peak memory. We are already stripping out the comments from the lua libs we ship on target (and leaving newlines to preserve line numbers) to mitigate this somewhat for drivers. If we do need something in every file can we use a condensed version that is only one or 2 lines? |
Maybe something like: |
|
Good questions. In the meantime luacheck has beef with you. |
59833ff to
df26b2e
Compare
This is a new feature in api v16 that allows for more memory savings by more efficiently lazily loading the subdriver. Subdrivers must have their can_handle function and their own subdrivers split out into separate modules to work with the new lazy loading api.
This is done to maintain consitency across the whole driver and set an example for other drivers to follow. The reduced source file comment should also help with memory savings since comments do affect memory of a driver when the driver code is being loaded into the Lua runtime.
170d8ab to
d42421a
Compare
|
This is ready for a review now. Testing has been done on my home hub (uses about 2 subdrivers), and then unit test coverage provides good coverage; it was relied on heavily during development. I also updated all the copyright comments in source files to match what we should be doing now as well as added it to the plethora of new files. |
We are considering adding a new way to lazy load subdrivers in 0.59.x, and this is what we would want to do to our zigbee switch driver to have it work with that functionality. The improvement is that we avoid requiring the base subdriver entirely to get access to the can handle and any nested subdrivers. So to convert our drivers subdrivers will all need to have a
can_handle.luafile and any base driver or subdriver that has subdrivers will need asub_drivers.luafile.