Skip to content

Conversation

@thewhobox
Copy link
Contributor

First attempt to fix permanently unloading a device according to thelsing/knx #144

@thewhobox
Copy link
Contributor Author

It's not yet tested.

It just marks tables as "wasUnloaded".
The loop in bau_systemB_device (and coupler) checks this and saves the time.
If the timeout reaches 5s it will call writeMemory().
If the device receives a memoryWrite/propertyWrite/routingTableWrite/... it will reset the timeout so it may not effect the parametration of the device.

@Phil1pp could you test it?

@Phil1pp
Copy link
Contributor

Phil1pp commented May 12, 2025

The timeout and the write is working, however it keeps triggering the write every 5000ms.
Some kind of writeDone or resetting of the WasUnloaded bits is missing.

@thewhobox
Copy link
Contributor Author

I resetted the flag WasUnloaded.
Should now only trigger once.

@Phil1pp
Copy link
Contributor

Phil1pp commented May 13, 2025

It looks good now. Thanks a lot.
I did several tests for unloading with and without physical address.
The unload was persistant after a device restart.
Also loading the application again was without a problem.

@thewhobox
Copy link
Contributor Author

Thanks for your testing.

@thelsing what do you think about the solution?

@thewhobox thewhobox marked this pull request as ready for review May 15, 2025 09:31
@thelsing
Copy link
Owner

My first idea would be to add the timeout to the Memory class instead. So on "ClearMemory" one would set a timeout and do a writeMemory after it's reached. This seems to be a bit more centralized if we clear memory for some other reason.

Does this seam reasonable?

@thewhobox
Copy link
Contributor Author

This would also be a good idea.
I moved it to the Memory Class.

I also added a reset of the timeout if there is write access to the memory. This will prevent write memory while the ets wants to load the application into the device (since it unloads every table before).

@Phil1pp
Copy link
Contributor

Phil1pp commented May 19, 2025

After you last change it doesn't work anymore because _saveTimeout is never set.
clearMemory() was just recently added by me and isn't involved in the unloading process.

You probably still have to check if TableObject state changed to LS_UNLOADED.

@thewhobox
Copy link
Contributor Author

Okay, i now moved it to freeMemory().
Whis will be called when the TableState changes from loaded to unloaded.
(and also when the table gets allocated but was not a nullptr)

https://github.com/thewhobox/knx/blob/9ebf5a572b73c7258f045758b7dc8b4ffaf151ec/src/knx/table_object.cpp#L253

@Phil1pp
Copy link
Contributor

Phil1pp commented May 27, 2025

I tested it again and it's working fine now.
Thx thewhobox

@thelsing
Copy link
Owner

It's a bit less obvious how this works than I'd like it to be, but it's good enough to merge.

@thelsing thelsing merged commit e2642b6 into thelsing:master Jul 21, 2025
7 checks passed
thewhobox pushed a commit to OpenKNX/knx that referenced this pull request Jul 29, 2025
cornelius-koepp added a commit to OpenKNX/knx that referenced this pull request Aug 14, 2025
Remove `clearMemory();` in Header as Result of Cherry-Pick
thelsing added a commit that referenced this pull request Oct 7, 2025
…nloding-fix

Fix for PR #316: Special Case Handling for Unload-Timeout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants