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

Add tim tm2 #27

Merged
merged 14 commits into from Aug 11, 2018
Merged

Add tim tm2 #27

merged 14 commits into from Aug 11, 2018

Conversation

flynneva
Copy link
Contributor

  • added TIM implementation
  • added TIM-TM2 msg

Copy link
Collaborator

@kartikmohta kartikmohta left a comment

Choose a reason for hiding this comment

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

@flynneva Thanks for the contributing! Can you please take a look at the comments below, once those are taken care of I can merge this in.

tio.c_cc[VMIN] = 1;
tcflush(fd, TCIFLUSH);
tcsetattr(fd, TCSANOW, &tio);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This block is not required since we set the correct mode for the serial port in the code just below this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the code right below this looks like it checks the boost version before setting the correct mode for the serial port. this code block above looks like it is meant to wait for at least one character from the serial port before returning from read....

Have you tested it without this block above to make sure that it is not needed? With different versions of boost?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like this block of code is from this comment: #20 (comment), so I'm fairly confident about it.
The Boost version check was added since Boost sets the correct parameters when initializing the serial port in versions >= 1.66.

// if (enabled["rxm_sfrb"])
// gps.subscribe<ublox_msgs::RxmSFRBX>(boost::bind(
// publish<ublox_msgs::RxmSFRBX>, _1, "rxmsfrb"), kSubscribeRate);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this function be cleaned up a bit? Lots of commented out code here.

Copy link
Contributor Author

@flynneva flynneva left a comment

Choose a reason for hiding this comment

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

-removed commented out bits of code
-answered question about port settings

Copy link
Contributor Author

@flynneva flynneva left a comment

Choose a reason for hiding this comment

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

removed section as requested

@@ -285,7 +274,7 @@ bool Gps::configReset(uint16_t nav_bbr_mask, uint16_t reset_mode) {

bool Gps::configGnss(CfgGNSS gnss,
const boost::posix_time::time_duration& wait) {
// Configure the GNSS settings
// Configure the GNSS settingshttps://mail.google.com/mail/u/0/#inbox
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like a copy paste by mistake here.

Copy link
Collaborator

@kartikmohta kartikmohta left a comment

Choose a reason for hiding this comment

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

Just one last change as mentioned in the comments below and then I can merge. Thanks for the contribution!

nh->param("publish/rxm/raw", enabled["rxm_raw"], enabled["rxm"]);
if (enabled["rxm_raw"])
gps.subscribe<ublox_msgs::RxmRAWX>(boost::bind(
publish<ublox_msgs::RxmRAWX>, _1, "rxmraw"), kSubscribeRate);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably want to keep the RXM-RAWX message option

nh->param("publish/rxm/sfrb", enabled["rxm_sfrb"], enabled["rxm"]);
if (enabled["rxm_sfrb"])
gps.subscribe<ublox_msgs::RxmSFRBX>(boost::bind(
publish<ublox_msgs::RxmSFRBX>, _1, "rxmsfrb"), kSubscribeRate);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably want to keep the RXM-SFRBX message option

Copy link
Contributor Author

@flynneva flynneva left a comment

Choose a reason for hiding this comment

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

made changes as requested (added back rxm msgs)

@kartikmohta kartikmohta merged commit fc92537 into KumarRobotics:master Aug 11, 2018
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