Hi all,
I have continued with my refactorings and mostly done:
* Remove global state (e.g. the global lists for ta, sba, tbf, bts * Add back-pointers to tbf, trx, pdch which will allow to kill methods that take trx, ts as parameters. * Started to use C++ classes and functions. * Stop poking of internals from other classes areas (gprs_rlcmac_data is still in front of me and the most hairy part).
I will probably need to spend another 40h on this code to finally have structure in it. Once this is done I will add counters and rate counters, improve the VTY inspection... and then I can search for the actual defects we are experiencing.
While doing the refactorings I noticed that both pcu_l1_if.cpp and sysmo_sock.cpp have different ways to reset the BTS/PCU state. Both of them have different issues and are both incomplete. It is the perfect example where structure would have saved timed and made the code more reliable at the same time.
I am going to work on gprs_rlcmac_data.cpp and while my refactorings so far where low risk.. the risk will increase when touching and structuring the above, e.g. it is hard to judge if the difference in the code-clones is on purpose or actually a bug. There will be only one way to find it out though.
I am afraid that I have 52 commits compares to master and I am only half way done. Review and changing directions will be quite difficult at this point in time.
holger
Holger Hans Peter Freyther wrote:
I am going to work on gprs_rlcmac_data.cpp and while my refactorings so far where low risk.. the risk will increase when touching and structuring the above, e.g. it is hard to judge if the difference in the code-clones is on purpose or actually a bug. There will be only one way to find it out though.
dear holger,
the TBF assignment process, the time slot allocation algorithm and the RLC/MAC data transfer were the most complicated parts of the PCU for me. the data transfer seems to be critical, but breaking things there might be easy to find out. in most cases it just doesn't work anymore. once you have cleaned up the data transfer part, i will test all your changes (at good coverage and at border of coverage) and review the debug output.
regards,
andreas
Hi Holger,
I have merged all patches, which you sent to mailing list previously and also looked through the code of zecke/features/clean-up branch. You have done really great work and as I think all structure changes have the right direction. As you mentioned, refactoring of some parts of the code is risky, but when we use right code structure, it is more easy to find problems. Also I think that after the end of refactoring, we should implement set of tests, which will help us to find bugs and problems.
I have one question about C/C++ using in osmo-pcu project. What I see, that now the code migrates to C++, but previously we mainly used C. As the result now osmo-pcu code is C and C++ mix, but I think that we should use only one as basic. I personally prefer to use C++ in osmo-pcu. What do you think about it? Do you plan to migrate the most parts of the code to C++?
2013/10/20 Holger Hans Peter Freyther hfreyther@sysmocom.de:
Hi all,
I have continued with my refactorings and mostly done:
- Remove global state (e.g. the global lists for ta, sba, tbf, bts
- Add back-pointers to tbf, trx, pdch which will allow to kill methods that take trx, ts as parameters.
- Started to use C++ classes and functions.
- Stop poking of internals from other classes areas (gprs_rlcmac_data is still in front of me and the most hairy part).
I will probably need to spend another 40h on this code to finally have structure in it. Once this is done I will add counters and rate counters, improve the VTY inspection... and then I can search for the actual defects we are experiencing.
While doing the refactorings I noticed that both pcu_l1_if.cpp and sysmo_sock.cpp have different ways to reset the BTS/PCU state. Both of them have different issues and are both incomplete. It is the perfect example where structure would have saved timed and made the code more reliable at the same time.
I am going to work on gprs_rlcmac_data.cpp and while my refactorings so far where low risk.. the risk will increase when touching and structuring the above, e.g. it is hard to judge if the difference in the code-clones is on purpose or actually a bug. There will be only one way to find it out though.
I am afraid that I have 52 commits compares to master and I am only half way done. Review and changing directions will be quite difficult at this point in time.
holger
--
- Holger Freyther hfreyther@sysmocom.de http://www.sysmocom.de/
=======================================================================
- sysmocom - systems for mobile communications GmbH
- Schivelbeiner Str. 5
- 10439 Berlin, Germany
- Sitz / Registered office: Berlin, HRB 134158 B
- Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte
On Mon, Oct 28, 2013 at 03:16:34PM +0400, Ivan Kluchnikov wrote:
As you mentioned, refactoring of some parts of the code is risky, but when we use right code structure, it is more easy to find problems. Also I think that after the end of refactoring, we should implement set of tests, which will help us to find bugs and problems.
with the manual toying around my E71 can open webpages. By cheer luck I spotted a "bts->bts = bts->bts" (which should have been a (tbf->bts = bts). But so far it is okay.
The main issue is that I have now around 90 commits. It will be not nice to review and changing direction will be difficult as well.
But e.g. my renaming of tfi/tlli in the code has lead me to create a tbf_name() function that I use for logging. And I found places that could have silently changed the tlli.
My selfish proposal would be to merge my branch to master and continue the clean-up there once some people having done smoke "testing" on it.
I have one question about C/C++ using in osmo-pcu project. What I see, that now the code migrates to C++, but previously we mainly used C. As the result now osmo-pcu code is C and C++ mix, but I think that we should use only one as basic. I personally prefer to use C++ in osmo-pcu. What do you think about it? Do you plan to migrate the most parts of the code to C++?
yes, to a very tiny subject of C++. E.g. I don't want to use stl and maybe only virtual functions when creating a GPRS TBF and a EGPRS TBF.
2013/10/28 Holger Hans Peter Freyther hfreyther@sysmocom.de:
The main issue is that I have now around 90 commits. It will be not nice to review and changing direction will be difficult as well.
But e.g. my renaming of tfi/tlli in the code has lead me to create a tbf_name() function that I use for logging. And I found places that could have silently changed the tlli.
My selfish proposal would be to merge my branch to master and continue the clean-up there once some people having done smoke "testing" on it.
Ok, let me try to test new code, after that I think we can merge it to master to speed up code debugging and testing.
osmocom-net-gprs@lists.osmocom.org
-
Andreas Eversberg -
Holger Hans Peter Freyther -
Ivan Kluchnikov