Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

correct callLog option in docs/CONFIGURE.md to true to match code #824

Merged
merged 2 commits into from
Jun 15, 2023

Conversation

rosecitytransit
Copy link
Contributor

I was going to change the default in the code to false, but then I realized that audioArchive defaults to true and am thinking callLog should match that. Otherwise, I should have included this in my other PR.

You want me to also add an entry for your tempDir option? I like your idea of having it in memory, it'll save hard drive wear, as well as address the issue of using a networked drive. The only downside being if trunk-recorder closes uncleanly, the current transmissions are lost.

But is it compatible with the transmissionArchive option for those who want to keep the files? How about making the option be "transmissionDir", with the net result of not needing to copy the finished file once complete? I'm guessing the combined, uncompressed WAV file could be put in the transmissionDir since most people wouldn't want to keep that either.

@robotastic
Copy link
Owner

ohhh.... good point, I think the current approach breaks the transmissionArchive option. I will go fix that. Of course I am not sure how many people use that feature. I will just move the Transmission Files to the captureDir if it is set.

I am going to keep the Temp Dir for now, because a plugin may want to use it for other purposes too.

@rosecitytransit
Copy link
Contributor Author

rosecitytransit commented Jun 4, 2023

I will just move the Transmission Files to the captureDir if it is set.

My thought is if transmissionArchive or callLog is true, use the captureDir, but if not it's safe to use a tempDir. Or have the tempDir default be the captureDir, with a suggestion to change it to memory.

Also, if I wasn't clear, I meant just changing the name "tempDir" to "transmissionDir" since I think that's what it would be (though I guess it wouldn't have the /year/month/day structure, and if not, then I don't think it needs to be created each call:

time_t work_start_time = d_start_time;
std::stringstream temp_path_stream;
tm *ltm = localtime(&work_start_time);
// Found some good advice on Streams and Strings here: https://blog.sensecodons.com/2013/04/dont-let-stdstringstreamstrcstr-happen.html
temp_path_stream << d_current_call_temp_dir << "/" << d_current_call_short_name;
std::string temp_path_string = temp_path_stream.str();
boost::filesystem::create_directories(temp_path_string);
)

But regarding this PR. do you want an entry in CONFIGURE.MD for it?

@rosecitytransit
Copy link
Contributor Author

@robotastic since I see you created an entry for tempDir, you want to go ahead and merge this in? Also PR 482 should be ready to go.

@robotastic
Copy link
Owner

Awesome! I added an entry for tempDir, so we shoudl be good there.

@robotastic robotastic merged commit 77d6b59 into robotastic:master Jun 15, 2023
1 check passed
@robotastic
Copy link
Owner

Sorry - missed the comment in there. I think having a general TempDir could be useful in case a Plugin wants to use it for things beside transmissions.

@rosecitytransit
Copy link
Contributor Author

rosecitytransit commented Jun 15, 2023

Thanks for getting to these! I was about to send you a reminder e-mail. Regarding tempDir, I'm liking the idea to use captureDir if tempDir is unset and suggest in CONFIGURE.md to use memory. The only issue would be those who want to save transmissions to slow storage. But it would would allow the old way, prevent upgrade issues, eliminate the need to copy files, and not lose what data there is on unclean closes. The combined WAV files could also be put in tempDir if they're not going to be kept, and even the compressed files if audioArchive is false.

When I get time, I'd be willing to submit a PR that does that if you're interested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants