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.

Sign up for GitHub

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

Disable rosout.log by using environment variable #1425

Merged
merged 9 commits into from
Aug 8, 2018
Merged

Disable rosout.log by using environment variable #1425

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8800c38
Disable rosout.log by using environment variable
yli-cpr Jun 6, 2018
cb024d9
Add dependency for boost
yli-cpr Jun 11, 2018
0c2dfdb
add boost include dirs in cmake
yli-cpr Jul 10, 2018
c29494b
add 'REQUIRED1
yli-cpr Jul 10, 2018
9dc99a0
as long as the env is defined, treat it as true for unknown values
yli-cpr Aug 7, 2018
0e18b2d
remove boost dependency
yli-cpr Aug 8, 2018
2b9f03d
revert unrelated changes
dirk-thomas Aug 8, 2018
bdc3055
revert unrelated changes
dirk-thomas Aug 8, 2018
7dfc095
trailing operators
dirk-thomas Aug 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions tools/rosout/rosout.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* POSSIBILITY OF SUCH DAMAGE.
*/

#include <boost/algorithm/string.hpp>
#include <cstring>
#include <cstdlib>

Expand Down Expand Up @@ -82,10 +83,8 @@ class Rosout

void init()
{
bool disable_file_logging = false;
node_.getParamCached("/rosout/disable_file_logging", disable_file_logging);

if (!disable_file_logging)
const char* disable_file_logging = getenv("ROSOUT_DISABLE_FILE_LOGGING");
if (!disable_file_logging || !boost::iequals(disable_file_logging, "true"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is new to me. Other than the allocation, is there any advantage to this over just instantiating a throwaway std::string and comparing that directly?

Copy link
Contributor Author

@yli-cpr yli-cpr Jun 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::string cannot be constructed with nullptr. So anyway, checking for null is required. I first tried std::string and got an exception complaining about nullptr

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, makes sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The case insensitive comparison to true seems to be rather restrictive. I would suggest the opposite and if the string is empty or 0 (maybe "false", "off"?) consider the value to be false, otherwise true.

{
handle_ = fopen(log_file_name_.c_str(), "w");

Expand Down