hi holger,

* We introduce a new API function that will handle the specifics of the USSD packing (last 7bit unused, last 7bit <CR>). The signature could be:

int gsm_7bit_encode_ussd(uint8_t *result, const char *data, int *octets_written); int gsm_7bit_decode_ussd(char *decoded, const uint8_t *user_data, uint8_t length);

i just pushed a branch (jolly/7bit_ussd) i was working on the last hours. it is quite exactly what you prosed.

* Add a testcase for the above (e.g. '1234567\r' should result in a double \r\r). * Switch the USSD code in libosmocore over

also it includes a test case. the result is similar to your proposal. currently i pushed everything togehter without propper splitting and commit log message. after review, i can do that. best regards, andreas

Holger Hans Peter Freyther wrote:

On Sun, Jul 07, 2013 at 02:41:50PM +0200, Andreas Eversberg wrote:

...

hi holger,

Dear Andreas,

...

i just pushed a branch (jolly/7bit_ussd) i was working on the last hours. it is quite exactly what you prosed.

I don't think it needs much splitting my comments are:

* If it helps we can export 'shift' from the common function as well, maybe this simplifies your calculation.

maybe, it will, but this would require export over two functions and still requires some calculations. i think it is simpler to understand the code, when using the number of characters for calculation.

* I would write ('\r' << 1), it is more obvious than 0x1A and modern compilers propagate such constants.

* The decoding, it might be easier to verify this condition on the encoded data?

* In the encoding. If you change octets you also want to modify 'y' as the number of septets.

see my branch.

...

also it includes a test case. the result is similar to your proposal.

What I don't understand is.. why the first broken version if you know it is not correct? It didn't save any of your time and it wasted plenty of mine.

if i would have known that it is not correct, i would have already fixed it. also it was not clear to me that SMS end USSD encoding is a bit different.

Dear Holger, I think you have iterated the point about 'wasted time' sufficiently enough. I understand you being upset, and the point has been driven home to everyone very clearly. I take responsibility for merging the 7bit changes. The 'make check' was unfortunately only discovered immediately after the push, and I sent a notice to Andreas that this needs fixing. The discussion of 'stability' vs. 'merging new developments' is a difficult one, and each project has to find its way around that. Linux had parallell stable an development trees for a long time, but this requires a lot more additional effort and people who actually work on 'stable' with time and discipline (and a motivation to do so, which often results from commercial use). I would personally satte: 1) there is no excuse for not running 'make check' to apply existing test cases against changes that are proposed for merging. No exception here. 2) patches requested to be merged should generally be sent through the applicable mailing list before merging them to master. No merge requests via private e-mail, and not just by mentioning a branch but by posting the entire patch set, patch for patch (git-send-email). This gives them much more visibility to more people. 2) for 'master' I would still want to see a more relaxed attitude than only accepting changes for which comprehensive test cases exist. That is too extreme. We need to balance stability vs. innovation here. Master may break things here and there. It's not a release for production use. 3) We should discuss whether we actually would like to start making formall releases (as opposed to asking everyone to use git master), and/or a stable branch which will only cherry-pick changes from master based on certain guidelines. Regards, Haradl -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)

Harald Welte wrote:

We can establish:

* Andreas should have run 'make check' before asking me to merge * I should have run 'make check' ahead of the commit and not immediately after ;) My apologies for that. .. So as a result, we introduce a new rule / process:

Maybe consider running 'make check' in a pre-receive hook on the server. I'm a big fan of Gerrit because it allows such verification to run asynchronously after a push. Gerrit sits between the developer and the official git repository. Everybody pushes to Gerrit, not to the actual repo. (Yes, when using a workflow that depends on gitolite or gitosis access control becomes an issue with Gerrit.) //Peter

Hi,

Maybe consider running 'make check' in a pre-receive hook on the server.

Not so easy ... it's not good practice to just allow anyone with push privilege to execute code on the server like that. So a good system with proper separation etc ... would be non-trivial. Also, this would slow down pushes to a crawl since you'd need to actually build the thing and its dependencies ... Cheers, Sylvain

On Wed, Jul 10, 2013 at 05:52:16PM +0200, Harald Welte wrote:

as an alternative, offer something like check_patch.pl of Linux kernel to the developer, but which not only checks for indentation/coding style but whihc also does the 'make check'. More reasonable and no load/security on the server.

I think it is a social problem, so no technological solution exists. I will add redundancy and reply somewhere else with similar content but from my point of view one should do the following on a branch before asking it to be merged: 1.) rebase against origin/master 2.) Do not mix too many concerns, so maybe create different patch series 3.) go through each change for sanity checking, check the commit message (if the problem fixed is properly described and if a drunk guy in at 10:00 am will understand and have enough context what was attempted to be fixed, I learned this with WebKit and re-learned it with the paging changes in OpenBSC.. so through my own failure) 4.) Make sure that each commit passes make and make check (distcheck) 5.) Ask things to be merged. So we are all humans and we sometimes don't do the above, sometimes we are lucky and it doesn't get noticed. But in case it is not I think the following should be a social norm: 1.) Fix it 2.) Apologize 3.) Promise to be more careful in the future and attempt to really do it cheers holger

10.07.2013 19:07, Holger Hans Peter Freyther пишет:

* Create a script to create debian packages and push them to build.opensuse.org and let them build things for us...

That would be the best IMO - in addition to smoothing out commit process this will make it much easier to install and maintain everything. -- best regards, Max, http://fairwaves.ru

Hi Holger, On Wed, Jul 10, 2013 at 07:07:55PM +0200, Holger Hans Peter Freyther wrote:

In terms of technology. I see the following options:

* Use the above script and push all commits of a branch (we want to test each commit) to Osmocom test repository and have travis-ci (a Ubuntu VM...) build it and we feed some info back.

If at all, I'm more in favor of client-side builds. Something as simple as 'take this branch, use the kernel coding style check_patch script, build each and every commit (locally) and return some status, preferrably by simply sending mail to the user himself. Then that process can be run in background without monitoring output on the console.

* We use gerrit for things we want to stage. Gerrit and a Jenkins would cooperate. There is ACL and user management. Everybody could signup with a OpenID... we could setup a VM with snapshot.. or limit the people that may push..

I'm really against such complex technology. That's not the kind of project I want to be involved in. And I hate web interfaces, they are always slow, require on-line connectivity and the use of a mouse. E-mail is so much more convenient.

* Create a script to create debian packages and push them to build.opensuse.org and let them build things for us...

sorry for being the naysayer again, but I reall dislike all this 'cloud' approach. I'd prefer to keep things simple and local, don't add external dependencies, etc. Regards, Harald -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)

Hi Holger, On Thu, Jul 11, 2013 at 07:52:08AM +0200, Holger Hans Peter Freyther wrote:

I classified this as a social problem because if someone is repeatable not willing to run make and make check on the last commit, I doubt that he will run an extra script as part of his development process.

I would definitely [want] to use such a script.

From a technical point the above makes sense. Where would you keep the script? Libosmocore and install it as part of it? A new repo?

libosmocore seems fine. -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)