Here's a few refactoring proposal that I'd like to get done. But before I invest too much time in them I'd like to be sure there are no objections ... (I hate implementing stuff for nothing).
* Split layer23/src into - common: what makes liblayer23, purely generic stuff that needs binding together to define an app - mobile: The real complete mobile application - misc: The test applications and their support code layer23 (which I'd rename bcch_dump since that's what it does), echo_test, bcch_scan)
* Move away from dispatching L1CTL message using signals ...
Only L1CTL works like that and I just can't figure out why ... we read the l1ctl messages just to move their data into another structure to redispatch them to a handler ...
l1ctl.c should just contain: - The l1ctl_tx_XXX primitives - A function for the 'application' to register it's own handler for L1CTL messages. - The rx call back to be called by lower layer when a l1ctl frame is received. This one would get the frame, set the hdr pointers, do basic checks, possibly allow signal dipatching for the RESET signal (which is kinda special), and then just hand the frame to the registered handler.
* Split lapdm.c : Currently it contains lapdm code and RSLms. They should be split and lapdm code should not be tied to a specific usage but be 'instantiated'/configured in each app as they see fit. Some app might need to track more than 2 lapdm channels ... - lapdm.c would be in common/ (more like an utility lapdm library than 'business' code) - rslms.c would probably be in mobile/
* Rework the app in misc/ to remove the over-engineered bits. It's clear from the sources that layer23 intended to become the full featured phone and as such, is split in a bunch of elements for ... not much in the end since Jolly restarted from scratch, splitting better. So I'd like to simplify as much as possible those apps while still separating 'utilities' function (like dump_bcch) to allow re-use between them.
Theses are only the first few steps. My goal is to make it more easy to write small test applications and to clearly separate that from the full featured / layered mobile implementation while still sharing the relevant 'library type' code.
Cheers,
Sylvain
Hi Sylvain,
On Sat, Jul 24, 2010 at 10:53:07PM +0200, Sylvain Munaut wrote:
Here's a few refactoring proposal that I'd like to get done. But before I invest too much time in them I'd like to be sure there are no objections ... (I hate implementing stuff for nothing).
- Split layer23/src into
- common: what makes liblayer23, purely generic stuff that needs
binding together to define an app
- mobile: The real complete mobile application
- misc: The test applications and their support code layer23 (which
I'd rename bcch_dump since that's what it does), echo_test, bcch_scan)
Sounds fine with me. If Andreas doesn't mind, I'm happy to merge a branch containing this split
Move away from dispatching L1CTL message using signals ...
Only L1CTL works like that and I just can't figure out why ... we
read the l1ctl messages just to move their data into another structure to redispatch them to a handler ...
l1ctl.c should just contain:
- The l1ctl_tx_XXX primitives
- A function for the 'application' to register it's own handler for
L1CTL messages.
- The rx call back to be called by lower layer when a l1ctl frame
is received. This one would get the frame, set the hdr pointers, do basic checks, possibly allow signal dipatching for the RESET signal (which is kinda special), and then just hand the frame to the registered handler.
ACK from me, too.
- Split lapdm.c : Currently it contains lapdm code and RSLms. They
should be split and lapdm code should not be tied to a specific usage but be 'instantiated'/configured in each app as they see fit. Some app might need to track more than 2 lapdm channels ...
- lapdm.c would be in common/ (more like an utility lapdm library
than 'business' code)
- rslms.c would probably be in mobile/
I don't have a strong feeling about this, but if you want to do that work, feel free :)
- Rework the app in misc/ to remove the over-engineered bits. It's
clear from the sources that layer23 intended to become the full featured phone and as such, is split in a bunch of elements for ... not much in the end since Jolly restarted from scratch, splitting better. So I'd like to simplify as much as possible those apps while still separating 'utilities' function (like dump_bcch) to allow re-use between them.
This also sounds very reasonable from my point of view.
Hi,
- Split layer23/src into
- common: what makes liblayer23, purely generic stuff that needs binding together to define an app - mobile: The real complete mobile application - misc: The test applications and their support code layer23 (which I'd rename bcch_dump since that's what it does), echo_test, bcch_scan)
Sounds fine with me. If Andreas doesn't mind, I'm happy to merge a branch containing this split
Ok, this is done. It's waiting in my pending branch with a bunch of other fixes. It requires a libosmocore update as well.
(I moved a func to libosmocore, which required to update it, which bringed the msgb checks, which broke the target build and required fixing, it also showed some bugs in fw and layer23 code that required fixing as well ...)
Andreas ?
The L1CTL stuff is next and then the test apps. The lapdm will probably wait as I don't have an immediate need for it.
Cheers,
Sylvain
On Tue, Jul 27, 2010 at 09:02:02PM +0200, Sylvain Munaut wrote:
Hi,
- Split layer23/src into
- common: what makes liblayer23, purely generic stuff that needs binding together to define an app - mobile: The real complete mobile application - misc: The test applications and their support code layer23 (which I'd rename bcch_dump since that's what it does), echo_test, bcch_scan)
Sounds fine with me. If Andreas doesn't mind, I'm happy to merge a branch containing this split
Ok, this is done. It's waiting in my pending branch with a bunch of other fixes. It requires a libosmocore update as well.
I'll wait for some feedback from Andreas. If he either agrees or doesn't care, then I'll merge it.
(I moved a func to libosmocore, which required to update it, which bringed the msgb checks, which broke the target build and required fixing, it also showed some bugs in fw and layer23 code that required fixing as well ...)
I've merged your pending libosmocore changes to libosmocore.git master.
Hi,
I have some pending changes in steve-m/pending and steve-m/l1reset as well. The first branch is based on current master, and re-enables the gta0x images, the l1reset branch is based on sylvain/pending and adds a l1ctl_tx_reset_req() on startup of the layer2 applications.
Thus, you don't have to start the layer2 application before powering the phone anymore, and you can switch between different layer2 apps without rebooting the phone. The first case still works, since the layer1 reset ack is received anyway.
Regards, Steve
On Wed, Jul 28, 2010 at 03:22:54PM +0200, Steve Markgraf wrote:
Hi,
I have some pending changes in steve-m/pending and steve-m/l1reset as well. The first branch is based on current master, and re-enables the gta0x images,
I've applied this to master, thanks.
the l1reset branch is based on sylvain/pending and adds a l1ctl_tx_reset_req() on startup of the layer2 applications.
Thus, you don't have to start the layer2 application before powering the phone anymore, and you can switch between different layer2 apps without rebooting the phone. The first case still works, since the layer1 reset ack is received anyway.
Sounds great, I first need to merge sylvain's branch, though.
Hi Steve and Sylvain,
On Wed, Jul 28, 2010 at 03:22:54PM +0200, Steve Markgraf wrote:
I have some pending changes in steve-m/pending and steve-m/l1reset as well. The first branch is based on current master, and re-enables the gta0x images, the l1reset branch is based on sylvain/pending and adds a l1ctl_tx_reset_req() on startup of the layer2 applications.
Ok, I've merged both sylvain/pending and steve-m/l1reset now.
baseband-devel@lists.osmocom.org
-
Harald Welte -
Steve Markgraf -
Sylvain Munaut