Openvario / variod

Daemon for autonomous e-vario
4 stars 8 forks source link

added support for multiple NMEA sentences from recv() #10

Closed bomilkar closed 4 years ago

bomilkar commented 4 years ago

This is the first part of testbench for variod. It is intended to test variod in a controlled environment without having to run sensord with sensor hardware. More details in README.txt

kedder commented 4 years ago

Is it possible to run it on regular laptop (i.e. not on openvario hardware) and hear the sound from variod (also running on the laptop)?

bomilkar commented 4 years ago

I run it on my OV machine as well as on my debian 10. make in bitbake variod-testing -c devshell compiles it for OV. A simple make does the trick in the local git repo.

I hear the sound on the OV machine, of course coming from variod. On the debian machine I run variod, too. I can't tell if variod produces a sound. I have no speaker connected. But why should it not beep there, too?

bomilkar commented 4 years ago

With my 2nd and 3rd commit I introduced a parameter to insert a minimum latency between 2 sentences. This is the "-l" command line argument followed by a number 0 ... 500 which stands for the latency in msec. When I run sensord_mock_20122d -l 0 -v -i ~/.xcsoar/logs/sample_flight_3.nmea I see after ~6000 sentence that not all sentences are reported by variod. This does not happen on the debian system, which is more powerful, of course. But I don't believe the CPU power makes the difference. Something else must be going on.

However, when I run sensord_mock_20122d -l 1 -v -i ~/.xcsoar/logs/sample_flight_3.nmea then all seem fine. The "-l 10" puts a usleep of 1 ms between every 2 sentences. That's the only difference. Even that 1 ms makes the difference.

What worries me: is sensord also putting a minimum delay between 2 sentences?? I need to get to the bottom of this. Unfortunately the busybox version of netcat on the OV machine doesn't support the "-l" option. Otherwise I would have listen on port 4353 nc -l -p 4353 to see what sensord_mock sends out. Any idea how else I can see what sensord_mock sends on port 4353 ??

bomilkar commented 4 years ago

I'm sorry guys, I meant to create a new branch for this last commit. It has many changes to address most (if not all) issues we have seen in the recent past. Namely https://github.com/Openvario/variod/pull/9 should now be fixed at the root cause.

Let me know if I should close this PR here and create 2 new PRs: one for the testbench tools and a second branch for the this last commit, which addresses the issues in variod. But before I close this PR here, please test to see if it really solves the pending issues with variod.

bomilkar commented 4 years ago

If you want to test it and want to "fast forward" to the binaries: https://www.dropbox.com/sh/8to1kfzkvp9rzs2/AAD_jKOjsYcQaHK2Kpl5iNNJa?dl=0

I compiled them today so it is 20124. You probably only need variod. It runs on the 20115 image: openvario-image-glibc-ipk-20115-openvario-7-CH070 Be careful though with xcsoar. The 20124 version is not fully compatible with the 20115 infrastructure. Unlike the previous builds it comes up with muted sound. Strange. But the unmute button works w/o problems.

linuxianer99 commented 4 years ago

@bomilkar : Please do not mix up the commits for fixes in variod with the testbench. It is also ok, to create more than one PR for different things..

If you create just one PR it makes it very difficult to review ...

bomilkar commented 4 years ago

All the testbench related commits have been reverted.

linuxianer99 commented 4 years ago

@bomilkar : OK. So please rebase. All the removing commits are not necessary for the main branch

bomilkar commented 4 years ago

Here is my log:

$ ./variod -f -c /opt/conf/variod.c
!! STAY in foreground !!
!! Using config file /opt/conf/variod.conf !!
Socket created ...
=========================================================================
Runtime Configuration:
----------------------
Vario:
  Deadband Low:                 -0.000000
  Deadband High:                0.000000
  Pulse Pause Length:           12288
  Pulse Pause Length Gain:      0.400000
  Base Frequency Positive:      500
  Base Frequency Negative:      500
Speed to fly:
  Deadband Low:                 -2.000000
  Deadband High:                2.000000
  Pulse Pause Length:           12288
  Pulse Pause Length Gain:      0.400000
  Base Frequency Positive:      500
  Base Frequency Negative:      500
=========================================================================
Connecting to pulseaudio...
Connection to pulseaudio established.
Waiting for incoming connections after 0 sentences from sensors ...
Connection accepted, reset sensor sentence counter
Request settings: $POV,?,RPO,MC*19
Get Polar IPO: 0.001558, -0.061671, 1.154286
Get Polar RPO: 0.001558, -0.061671, 1.154286
Get Ballast Value: 1.000000
Get Ballast Value: 1.013000
Get Ballast Value: 1.026000
Get Ballast Value: 1.039000
Get Ballast Value: 1.052000
Get Polar RPO: 0.001519, -0.061671, 1.183812
Get Ballast Value: 1.065000
Get Polar RPO: 0.001510, -0.061671, 1.191079
Get Ballast Value: 1.078000
Get Ballast Value: 1.091000
Get Polar RPO: 0.001492, -0.061671, 1.205482
Get Bugs Value: 0.990000
Get Polar RPO: 0.001507, -0.062294, 1.217659
Get Bugs Value: 0.980000
Get Bugs Value: 0.970000
Get Bugs Value: 0.960000
Get Bugs Value: 0.950000
Get Polar RPO: 0.001571, -0.064917, 1.268928
Get McCready Value: 0.500000
Get McCready Value: 0.400000
Get McCready Value: 0.300000
Get McCready Value: 0.400000
Get McCready Value: 0.500000
Get McCready Value: 0.600000
Get McCready Value: 0.700000
recv(connfd,...) return: 0
Waiting for incoming connections after 4893 sentences from sensors ...

I added a line fprintf(stderr, "recv(connfd,...) return: %d\n",read_size); below the loop while ((read_size = recv(connfd, client_message, sizeof(client_message), 0 )) > 0 ) { . It gets there when I kill the process which acts on behalf of sensord. At this time 4893 sentences were received and parsed. As I said: the only way to get there is to kill the process which feeds into the socket. So if you get there with sensord, it means sensord is either dying or closing the connection. Right?

I can kill and start the supplying process as often as I want to:


Connecting to pulseaudio...
Connection to pulseaudio established.
Waiting for incoming connections after 0 sentences from sensors ...
Connection accepted, reset sensor sentence counter
Request settings: $POV,?,RPO,MC*19
Get Polar RPO: 0.001571, -0.064917, 1.268928
Get McCready Value: 0.700000
recv(connfd,...) return: 0
Waiting for incoming connections after 191 sentences from sensors ...
Connection accepted, reset sensor sentence counter
Request settings: $POV,?,RPO,MC*19
Get Polar RPO: 0.001571, -0.064917, 1.268928
Get McCready Value: 0.700000
recv(connfd,...) return: 0
Waiting for incoming connections after 167 sentences from sensors ...
Connection accepted, reset sensor sentence counter
Request settings: $POV,?,RPO,MC*19
Get Polar RPO: 0.001571, -0.064917, 1.268928
Get McCready Value: 0.700000
recv(connfd,...) return: 0
Waiting for incoming connections after 49 sentences from sensors ...
iglesiasmarianod commented 4 years ago

It only gets there with 0 or -1. What means Variod was not the problem from the begining. My problems with STF and Mute commands had nothing to do with parsing. They had to do with disconnections. Don't misunderstand what I say please, robustness is welcome, but we need to solve this problem first, and I don´t think we can emulate what we don´t know why is failing and how it is failing. If I mock sensord like you I don't get errors. The only way to emulate this behaviour is if you sensor bot was able to emulate exactly what the actual sensord does. Or if sensord acted as your bot does. Why sensord is now failing and not failing in armstrong? Don't know yet.

bomilkar commented 4 years ago

You have a point: my sensor bot can't emulate exactly what sensord does. But it can emulate a well behaved sensord. And it gives you the knobs to explore what variod does if you change the parameters. For instance, if you reduce the delay between 2 sentences to 0. It helps tracking down the issue on the variod side. Once that's cleared there is a better chance to pin down the issues on the sensord side. Divide & conquer.

iglesiasmarianod commented 4 years ago

You are asusming what "well behaved" means. I still don't know how sensord is supposed to work from design. That's why I'm trying to reach the creators. When we know how it is supposed to work, an emulator would be possible.

bomilkar commented 4 years ago

@linuxianer99 what are we waiting for? Any opinion??

kedder commented 4 years ago

@bomilkar, I'll check this out tomorrow, too late for deep digging in.

linuxianer99 commented 4 years ago

@bomilkar : Please rebase the code tree. There are lots of commits which are changing things twice. I want to have a clear and easy to read commit history in the main repos. That's the thing i am wating for ;-)

bomilkar commented 4 years ago

@linuxianer99 : I could rebase the code that wouldn't make it easier to understand, I guess. Unless you object I propose to close this PR and make a new one with a couple of commits. Each change with it's own commit. Agreed?

linuxianer99 commented 4 years ago

@bomilkar That's not about understanding the code but about the useless commits: Five of the 11 commits are removing stuff which was added before which have also nothing to do with the name of the PR:

Revert "clean up NMEA sentence passing and socket stability issues" …

removing testbench stuff from this branch

2nd removing testbench stuff from this branch

3nd removing testbench stuff from this branch

4th removing testbench stuff from this branch

So please put only the relevant commits in this PR.

bomilkar commented 4 years ago

@kedder : I guess you must have missed the discussion earlier today. The intention is to close this PR because it became very convoluted over time. In a message to @linuxianer99 I suggested to close, clean up and open new. I was just waiting for his OK. Hence I will close it now to reduce further confusion. I'm planning a new PR in the next couple of days. I'm working on it.... In fact, most of what you complain about has already been addressed. Sorry for the confusion.

kedder commented 4 years ago

@bomilkar that's fine, no problem. You don't need to close PRs though, you can just force-push into the same branch, and PR will get updated. Feel free to open new one though, if that sounds confusing.

bomilkar commented 4 years ago

@kedder it's there now: https://github.com/Openvario/variod/issues/14