various enhancements (closes most of #456, closes #451) #470
Conversation
this is really meant for unit-unit calls, and where the new transmission is reversed to keep transmissions together, message.talkgroup and message.source could be switched here (not doing so if real talkgroups and radio IDs can overlap)
|
Thanks @rosecitytransit !! This looks like it should clean up the silence/idle frame stuff that had been lurking in the code. I am a little hesitant to add in the daily-log functionality. I am trying to keep the core of TR focused. Would it be possible to something similar as part of the upload-script? I would process the associated .json file and append a line to the end of the log file. This seems like it would be a great plugin. I am going to focus on getting the plugin system working next. It would allow for these types of customization. Can you separate out the daily-log changes as a separate PR? One thing I will have to thoroughly double check in the future, is how TR and OpenMHZ handle timezones and the concept of a day. TR uses local time I think and openmhz uses that time... I think. Can you tell me a little more about Voice Chan vs Control Chan source IDs? Do they differ ever? Would there be a reason for tracking them separately? For future PRs, it is easiest if you keep them focused on a single set of changes. It makes it easier to see what parts are related... and I can also tackle small ones quicker. :) Thanks for trying to make TR better!! |
|
I do apologize that I waited to submit everything at once. I know I should have done it as I went along. As can be seen here or here, I had some of it done 25 days ago. I am willing to reset and rebuild the commits again (I have the patches created/saved) and either leave out the unwanted ones, or just submit them piece-by-piece. As for control channel vs. voice frames radio IDs, I could send you raw signals from each to show how I don't get many of them on the control channel. Also, one of the reasons why I wanted to include the daily logs is that the live Web page uses them so as to not have to read 100's of individual JSON files. But given that I'm not sure I'm adding to them in the best way, I'd be fine with submitting the function separately. |
|
Thanks @rosecitytransit ! I went through and tried to slice it up in to changes that could be approved right away vs others that need more research. I want to to try and figure out why the Source IDs are not in the Control Channel but are in the Voice Channel. I like the daily log concept, but I would like to find a more flexible way of doing it. People may want different time scales of summerization or for it to end up somewhere besides a text file. Having it be part of the Upload Script would give that flexibility, but ultimately it would be a great use of the Plug-In system... I will go focus on getting that working. Can you go up the PR with the features that I tagged up above to include? |
|
To clarify before I rebuild things: leave out b69d64d modify 7a3447b to remove daily log parts (and maybe put controlWarnUpdate README entry with it's commit) leave out 1bd55e1 leave out and look into 9792d95 leave out 1bd55e1 leave out d210033 since it uses the daily log also what's your thought on 2108519 ? I only really need it due to lack of control channel updates for unit-unit calls, but otherwise it's not good for transmission mode (nor is the re-tune function). |
|
That is much clearer - I didn't realize they were sort of separated by Commit. I miseed 2108519 - lets add that into the PR. It seems like it would only add functionality. It is probably worth reinvestigating how Errors are calculated and recorded. I think there is some better code that was added to OP25 that could be used in the future. What you propose above looks good to me. Can you submit a PR based upon that? |
but should it (and timeDiff and statusTimeDiff) be ints instad of floats?
may not be implemented in all places
if you don't care about post-voice data, (un)comment lines in p25p1_fdma.cc as noted
also ACKRESP->ACKNOWLEDGE
(goes w/commit 2d6a954)
some people, like me, are recording from systems they are not uploading could also ensure that key is correct length
goes w/commit 366cd3b also add some voice processing debug messages
|
OK, I reset my branch and (hopefully correctly) re-added just the commits you were OK with. It should be ready to go now. I'll add the other things (daily log, live Web page, etc in other PRs after you merge this one). I'll also get some trunk messages w/frequency/talk group of 0 for you. |
|
Looks like patch failure got saved in the code. Thanks for noticing it and
I'll look into it tonight.
…On Wed, May 19, 2021, 3:20 PM E. Lee ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In trunk-recorder/systems/p25_parser.cc
<#470 (comment)>
:
> + message.duplex = duplex;
+ message.mode = mode;
+ message.priority = priority;
From the buildx @ 86689ce
<https://github.com/robotastic/trunk-recorder/runs/2624228538#step:7:3420>
:
#13 300.5 /src/trunk-recorder/systems/p25_parser.cc:456:22: error: 'duplex' was not declared in this scope
#13 300.5 456 | message.duplex = duplex;
#13 300.5 | ^~~~~~
#13 300.5 /src/trunk-recorder/systems/p25_parser.cc:457:20: error: 'mode' was not declared in this scope; did you mean 'modf'?
#13 300.5 457 | message.mode = mode;
#13 300.5 | ^~~~
#13 300.5 | modf
#13 300.5 /src/trunk-recorder/systems/p25_parser.cc:458:24: error: 'priority' was not declared in this scope
#13 300.5 458 | message.priority = priority;
#13 300.5 | ^~~~~~~~
@rosecitytransit <https://github.com/rosecitytransit> do these lines
<https://github.com/robotastic/trunk-recorder/pull/470/files#diff-f75ab21e8810348f73654b768283e40a0d9948d9a1d89c12cd73fd55c5b6cba4R442-R444>
need to be uncommented out?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#470 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFAB7DK63I3CA4KP33CTCRDTOQ2R3ANCNFSM446UFRAA>
.
|
|
should be good to go |
|
I believe it should be ʼtraceʼ because it is a repetitive system message.
I think i changed it setting up a system where I saw it.
…On Thu, May 20, 2021, 2:54 PM E. Lee ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In trunk-recorder/systems/p25_parser.cc
<#470 (comment)>
:
> } else if (opcode == 0x30) { //
- BOOST_LOG_TRIVIAL(debug) << "tsbk30";
+ BOOST_LOG_TRIVIAL(trace) << "tsbk30 TDMA SYNCHRONIZATION BROADCAST";
trace or debug?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#470 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFAB7DMSK2VO5B4XDVAG6MLTOWAINANCNFSM446UFRAA>
.
|
|
I was wondering about that and asked in the commit message "should it (and
timeDiff and statusTimeDiff) be ints instead of floats?"
…On Thu, May 20, 2021, 2:58 PM E. Lee ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In trunk-recorder/main.cc
<#470 (comment)>
:
> @@ -539,7 +539,9 @@ bool load_config(string config_file) {
config.log_dir = pt.get<std::string>("logDir", "logs");
BOOST_LOG_TRIVIAL(info) << "Log Directory: " << config.log_dir;
config.control_message_warn_rate = pt.get<int>("controlWarnRate", 10);
- BOOST_LOG_TRIVIAL(info) << "Control channel warning rate: " << config.control_message_warn_rate;
+ BOOST_LOG_TRIVIAL(info) << "Control channel decoding warnings (< messages/second): " << config.control_message_warn_rate;
+ config.control_message_warn_updates = pt.get<float>("controlWarnUpdate", 3.0);
In the property tree, you get controlWarnUpdate as a float, but don't you
describe it as an int in seconds?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#470 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFAB7DLHLYFOF57RLDBU7KLTOWA2FANCNFSM446UFRAA>
.
|
|
Good point on the Time Diff-ing being an int, vs a float. Not sure why I did it that way. Let's merge what you have in. We can can move those calculations over to be ints down the road. |
See list I worked from at #456 (also closes #451)
Notes:
Call::create_filename()that can be put in the daily log file/on the live Web page; it would allow calls to have a fixed row number based on start time (even if an earlier-started call ends/arrives later) and identify when calls are not listed on the Web page