Conventional systems with channels that can't be covered by any source don't get TGs numbered right

[leee]

My apologies for the mess clang-format has made to the PR overall change view.

Last night I set up a test trunk-recorder configuration with a bunch of conventional resources which used more than 1 SDR source. In my haste, I didn't give enough band edge spacing not just for roll off (keeping my sample rate and subsequent bandwidth tight is a personal preference), but also rate/decim (which ought to be documented, but I digress).

Cue my surprise when I get informed there seems to be weird off-by-one-or-more channel name/conventional TG# to frequency issue.

This is because of the way main.cc monitor_system() iterates through channels to be added in a conventional system. In particular, it only iterates talkgroups when a source has been found. If a source isn't found, then the system works the next channel to be added without incrementing talkgroup number.

This means if you have a channel(s) that doesn't have a source, your frequency array no longer lines up with your expectations like in your alphatags array (which by extension also applies to anywhere you're uploading as well).

Some additional infuriating bits:

1. reusing the talkgroup variable, not just for this incrementing integer, but also a Talkgroup object pointer.
2. having references to a CC when this is conventional (this is fine, I get that this is code reuse from the trunking portion of the setup)

These changes remove and clean up things that don't get used, and renames the for loop talkgroup increment variable to tg_interate_index. These also fix the primary issue by incrementing the talkgroup index whether or not a source has been found, as well as adding a channel_added variable with testing to provide user error warning on a channel that can't be recorded.

The reason this is an error and not a fatal is because there are trunk-recorder installations that don't completely cover all channels of a trunk(s), and run on best-possible recording ("Not Recording: no source covering Freq"). I figure systems that record conventional channels and wish to operate best-possible recording should receive similar warning (which is provided at startup).

Another problem: if a source squelch isn't set, then the program just segfaults. This should be made into an issue, and work on graceful exit. (The issue could also probably get rolled into the general issue of "make squelch and other configuration knobs system/channel-specific and not source-specific).

I'm groggy from being woken and earlier than typical, to report the testing environment, but it's the same host from my recent previous PRs, only changes made are to configuration files and the program. This PR is confirmed working with both:

• properly configured sources that have ample band edges and cover all channels
• and with improperly configured sources introducing uncoverable channels - error warning is properly provided.

Please let me know if there are any questions I can answer.

[robotastic]

This is a pretty busy PR. For future ones, can you separate out code formatting into a different PR? It makes it easier to track the substantive changes. People are also using different formatters, so the code flip-flops between styles. What formatter do you use? Happy to just standardize on one.

yea, that variable reuse is gross - looks like it came in here: edafb90

The assumption was that all conventional channels would have a source, but I can see cases like what you describe cropping up. Should a big error message pop up or should it exit when there is no source for a conventional channel? I could see cases where it could just issue a warning and someone may not notice that a conventional channel is not being recorded. Right now it will succeed if at least one system is added, maybe it should fail if some systems/channels can not be added. It would probably help catch a lot of configuration errors.

Hmm... squelch should get a default value of 0. Does it segfault if it is 0? There could be something else happening. Making squelch be system might be possible. The squelch value is used to configure the GnuRadio graph. It might be adjustable at run-time instead of requiring a stopping and starting of the graph. It might be possible to have the squelch set when an analog recorder is assigned to system. For conventional systems, this would be at config time, for SmartNet systems this would be when a recording is started.

[leee]

This is a pretty busy PR. For future ones, can you separate out code formatting into a different PR? It makes it easier to track the substantive changes. People are also using different formatters, so the code flip-flops between styles. What formatter do you use? Happy to just standardize on one.

I agree and apologize again :-) If you'd like, I can squash the real commits as one.

Would you object to a PR that runs clang-format over trunk-recorder?

I've just been using clang-format, but I will gladly use something else that everyone prefers, and future contributions can be informed to use that.

The assumption was that all conventional channels would have a source, but I can see cases like what you describe cropping up.

To clarify, do you mean the intent was that all conventional channels from one system would have the same source?

Should a big error message pop up or should it exit when there is no source for a conventional channel?

Yeah so this is where I'm going to defer that decision to you as the CODEOWNER.

What I can say is I know at least two people who use trunk-recorder who have "test" configuration files where they define a bunch of sources, but only have a few or just one of them actually connected. trunk-recorder currently considers this situation as non-fatal.

I could see cases where it could just issue a warning and someone may not notice that a conventional channel is not being recorded.

This is kicking the rock down the road, but I'm hoping once a plan for "enhanced logging" (#310) is fleshed out, that situations like this go there.

I'm not trying to lick that cookie problem, but for example, I was thinking of a logging system that ingests everything, including CC data, and users can pick out the data they want from that. For example, #181 does subscriber status tracking, but does that within the software. Instead, that feature could be a separate program that pulls CC data from logging. (In implementation though, systems data should be separate from trunk-recorder data.)

Non-breaking configuration errors like this one could be considered important for users to know about, but not a fatal error. The program ought to continue running. A separate error reporter would make this problem clear to the user.

Hmm... squelch should get a default value of 0. Does it segfault if it is 0? There could be something else happening.

I wasn't willing to dig in deep to look at why that was happening this morning. I figure that could be an issue for now and can be picked up later.

Making squelch be system might be possible. The squelch value is used to configure the GnuRadio graph. It might be adjustable at run-time instead of requiring a stopping and starting of the graph.

My understanding (as you say in the next quote) is squelch is set when starting a recorder (IIRC adding a squelch value adds a power squelch to the analog flowgraph?), and this doesn't need to be a dynamic thing.

It might be possible to have the squelch set when an analog recorder is assigned to system. For conventional systems, this would be at config time, for SmartNet systems this would be when a recording is started.

Right, and this is why I'm thinking if one is willing to make squelch be a system knob and not a source knob, might as well go all the way and make it possible for a channel squelch setting to overload a system squelch setting. At some point, tone squelch support will make its way into trunk-recorder, and configuration of channels will need to be more than just frequency.

[robotastic]

Let me give ClangFormat a try. That seems like it could be a good tool to standardize on.

I am not sure on the correct behavior for handling a system/conventional channel that is not covered by a source. Good to know that some folks are knowingly using configs where there may not be complete coverage. An option could be to have the default be to exit if a channel cannot be covered. This is how it works for a single system, so it should be consistent. I could a way to override it though with a commandline flag.

The challenge with having Squelch set at the system level is that if you had 2 SmartNet system with analog talkgroups, the recorders would be shared between the two systems. That means you can't lock in the squelch for each recorder, it needs to be set when a recorder is assigned to a system. Gnuradio lets you adjust this at runtime, so I think this could be done.

This is challenge in having per system modulation. Changing the modulation requires reconfiguring the GR graph, which requires a full stop/start. The solution here might be to create separate QPSK & FSK4 recorders for each source and systems would pull the digital recorders by modulation type.

Going further down this rabbit hole - it looks like GnuRadio has stabilized a bit and it is no longer that bad to start & stop the graph to make changes. It maybe possible to now dynamically create recorders and add them to the graph as needed. This might be a good branch/experiment to eval performance/stability. It would simplify things.

[leee]

Responding in reverse order:

This is challenge in having per system modulation. Changing the modulation requires reconfiguring the GR graph, which requires a full stop/start. The solution here might be to create separate QPSK & FSK4 recorders for each source and systems would pull the digital recorders by modulation type.

Right, the concept of a source having to be qpsk or fsk4 isn't a thing. You already know what modulation a system is operating with. Conventional systems are easy, you know what frequency and therefore what source they're using, and you create the recorder with the specified modulation. Trunking less so, since you currently operate on this concept of pools.

Probably the easiest way as you say is specifying number of qpsk and fsk4 recorders for each source and those many number of recorders are created and connected at startup. When a call comes in, instead of just hunting for an available recorder by frequency, also hunt by recorder type availability.

The onus then goes on the user to determine how many recorders of each type should be made available on each source. Given trunks don't frequently add additional system channels (turns out installing infrastructure is expensive and hard, especially to established hardware where you'd need to rebuild tx combiners, etc.), users probably already know all the talkpath frequencies and can calculate all this accordingly.

Going further down this rabbit hole - it looks like GnuRadio has stabilized a bit and it is no longer that bad to start & stop the graph to make changes. It maybe possible to now dynamically create recorders and add them to the graph as needed. This might be a good branch/experiment to eval performance/stability. It would simplify things.

I'm... hesitant on this. The time it takes to stop, make changes, and restart, will make you lose messages. Maybe this could be done if:

• you specify a minimum number of recorders of whatever types to be initially generated and connected
• users have a config switch to "halt and add additional recorders if they're needed" and disable this behavior
• trunk-recorder sees a call, which is within the frequency range of one of their sources, but not enough recorders, so act accordingly (an additional note below)
• users are then informed in logging that additional recorders were needed (which lets them know to increase the minimum number of recorders)

Right now trunk recorder operates on "start from the first source and work through your list of sources to find one with the frequency range you need". If you have sources with overlap, that isn't taken into account, so you might exhaust all the recorders on source n, but have recorders available on source n+1 that covers your frequency. Handling of these cases should be implemented.

The challenge with having Squelch set at the system level is that if you had 2 SmartNet system with analog talkgroups, the recorders would be shared between the two systems. That means you can't lock in the squelch for each recorder, it needs to be set when a recorder is assigned to a system. Gnuradio lets you adjust this at runtime, so I think this could be done.

Conventional channels are again easy here, setting squelch down even to a per-channel granularity is possible.

Pooled recorders for trunking makes this harder of course. Consider this though: when you're recording two analog SmartNet systems, with different connect tones, and trunk-recorder eventually has tone squelch capability. If you want tone squelched analog recorders for trunking, you will no longer be able to use one big pool of recorders since you can't change what tone they're squelching on during runtime.

I hate to say it, but thinking of individual separate pools for ( (source), (system which defines modulation and tones) ) might be worth considering.

This is a LOT of additional complexity, which I don't like ☹️

(Also, since this appears to be a topic deserving of its own issue, it might be worthwhile to continue discussion in a new issue and accept this PR separately)

[robotastic]

Agreeing on your analysis - another option in some scenarios is also to run multiple instances. There is a balance between trying to have a single instance that can handle every possible scenario vs multiple simpler instances. Using RTL-TCP, it maybe possible to multiplex a single SDR into multiple instances... been meaning to check that out.... it could also let me run a Docker version on my Mac...

Anyhow, back to the issue at hand. This change looks good. I will also go check out that Segfault and make it exit if a system can not be covered unless a flag is set.

I also like Clang Formatter. Going to go run it against everything and do a commit just for that.

[robotastic]

assuming the segfault for squelch has been addressed