Skip to content
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

Use an iterative method for expanding commands now #90

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dirkbaechle
Copy link

This merge request switches the methods "expand, buildByteCode and compileCommand" to using an iterative approach. I understand that the recursive approach used so far, looks more elegant at first glance. But it can give problems in the form of segfaults (see background story below) when trying to parse and process a large number of commands from a config file.
Switching to an iterative method introduces the "*All" wrapper methods to the source base (not so nice), but makes the overall process of parsing commands more robust (very nice).

Background: I'm currently starting to use vcontrold/vclient for monitoring and controlling my Vitodens200 (WBC2). Since I still don't know a few of the memory addresses, I wrote a Python script that can generate a "special" vito.xml. In this vito.xml I will define a single command "getVitoXXXX" for each address in a configurable range (see create_shell.py and create_vito.py in "scripts" folder of https://github.com/dirkbaechle/pyvctrl ). By checking all memory addresses before and after I changed the wanted values at the Vitodens200 directly, I can then do a simple diff to find the memory addresses by brute-force. When starting the vcontrold daemon with a vito.xml file for the addresses 0x2000-0x4000 (yes, that's a lot) the process crashes on my mini laptop (Samsung EE PC, 32bit, 2GB RAM, Linux Mint 19.3).
With the code from this branch vcontrold is running fine...

@speters
Copy link
Member

speters commented Aug 27, 2022

Thx, Dirk!
Please excuse the late reply. I hope to find some time to have a look at this in the next few days.

@speters
Copy link
Member

speters commented Dec 6, 2022

I would like to get this merged, but still had no feedback from other testers, as this does quite some alterations under the hood.

To whom it might concern: Please give this a try and provide some feedback.

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.

2 participants