Make GCodeSender more robust (keep more than one sent line) and fix a memory access problem in the asio write buffer

This commit is contained in:
Alessandro Ranellucci 2016-03-13 18:27:02 +01:00
parent ff0a947364
commit d5ff69b1aa
2 changed files with 66 additions and 30 deletions

View file

@ -26,6 +26,8 @@
std::fstream fs;
#endif
#define KEEP_SENT 20
namespace Slic3r {
namespace asio = boost::asio;
@ -201,12 +203,13 @@ void
GCodeSender::purge_queue(bool priority)
{
boost::lock_guard<boost::mutex> l(this->queue_mutex);
std::queue<std::string> empty;
if (priority) {
// clear priority queue
std::list<std::string> empty;
std::swap(this->priqueue, empty);
} else {
// clear queue
std::queue<std::string> empty;
std::swap(this->queue, empty);
this->queue_paused = false;
}
@ -308,7 +311,7 @@ GCodeSender::on_read(const boost::system::error_code& error,
std::getline(is, line);
if (!line.empty()) {
#ifdef DEBUG_SERIAL
fs << "<< " << line;
fs << "<< " << line << std::endl << std::flush;
#endif
// note that line might contain \r at its end
@ -335,16 +338,27 @@ GCodeSender::on_read(const boost::system::error_code& error,
// extract the first number from line
boost::algorithm::trim_left_if(line, !boost::algorithm::is_digit());
size_t toresend = boost::lexical_cast<size_t>(line.substr(0, line.find_first_not_of("0123456789")));
if (toresend == this->sent) {
if (toresend >= this->sent - this->last_sent.size()) {
{
boost::lock_guard<boost::mutex> l(this->queue_mutex);
this->priqueue.push(this->last_sent);
this->sent--; // resend it with the same line number
// move the unsent lines to priqueue
this->priqueue.insert(
this->priqueue.begin(), // insert at the beginning
this->last_sent.begin() + toresend - (this->sent - this->last_sent.size()) - 1,
this->last_sent.end()
);
// we can empty last_sent because it's not useful anymore
this->last_sent.clear();
// start resending with the requested line number
this->sent = toresend - 1;
this->can_send = true;
}
this->send();
} else {
printf("Cannot resend %zu (last was %zu)\n", toresend, this->sent);
printf("Cannot resend %zu (oldest we have is %zu)\n", toresend, this->sent - this->last_sent.size());
}
} else if (boost::starts_with(line, "wait")) {
// ignore
@ -370,7 +384,6 @@ GCodeSender::on_read(const boost::system::error_code& error,
}
}
}
this->do_read();
}
@ -382,7 +395,7 @@ GCodeSender::send(const std::vector<std::string> &lines, bool priority)
boost::lock_guard<boost::mutex> l(this->queue_mutex);
for (std::vector<std::string>::const_iterator line = lines.begin(); line != lines.end(); ++line) {
if (priority) {
this->priqueue.push(*line);
this->priqueue.push_back(*line);
} else {
this->queue.push(*line);
}
@ -398,7 +411,7 @@ GCodeSender::send(const std::string &line, bool priority)
{
boost::lock_guard<boost::mutex> l(this->queue_mutex);
if (priority) {
this->priqueue.push(line);
this->priqueue.push_back(line);
} else {
this->queue.push(line);
}
@ -420,11 +433,11 @@ GCodeSender::do_send()
// printer is not connected or we're still waiting for the previous ack
if (!this->can_send) return;
std::string line;
while (!this->priqueue.empty() || (!this->queue.empty() && !this->queue_paused)) {
std::string line;
if (!this->priqueue.empty()) {
line = this->priqueue.front();
this->priqueue.pop();
this->priqueue.pop_front();
} else {
line = this->queue.front();
this->queue.pop();
@ -437,18 +450,11 @@ GCodeSender::do_send()
boost::algorithm::trim(line);
// if line is not empty, send it
if (!line.empty()) {
this->do_send(line);
return;
}
if (!line.empty()) break;
// if line is empty, process next item in queue
}
}
// caller is responsible for holding queue_mutex
void
GCodeSender::do_send(const std::string &line)
{
if (line.empty()) return;
// compute full line
this->sent++;
std::string full_line = "N" + boost::lexical_cast<std::string>(this->sent) + " " + line;
@ -462,14 +468,39 @@ GCodeSender::do_send(const std::string &line)
full_line += "*";
full_line += boost::lexical_cast<std::string>(cs);
full_line += "\n";
asio::async_write(this->serial, asio::buffer(full_line), boost::bind(&GCodeSender::do_send, this));
this->last_sent = line;
this->can_send = false;
#ifdef DEBUG_SERIAL
fs << ">> " << full_line;
fs << ">> " << full_line << std::flush;
#endif
this->last_sent.push_back(line);
this->can_send = false;
if (this->last_sent.size() > KEEP_SENT)
this->last_sent.erase(this->last_sent.begin(), this->last_sent.end() - KEEP_SENT);
// we can't supply asio::buffer(full_line) to async_write() because full_line is on the
// stack and the buffer would lose its underlying storage causing memory corruption
std::ostream os(&this->write_buffer);
os << full_line;
asio::async_write(this->serial, this->write_buffer, boost::bind(&GCodeSender::on_write, this, boost::asio::placeholders::error,
boost::asio::placeholders::bytes_transferred));
}
void
GCodeSender::on_write(const boost::system::error_code& error,
size_t bytes_transferred)
{
this->set_error_status(false);
if (error) {
if (this->open) {
this->do_close();
this->set_error_status(true);
}
return;
}
this->do_send();
}
void
@ -502,6 +533,10 @@ GCodeSender::reset()
boost::this_thread::sleep(boost::posix_time::milliseconds(200));
this->set_DTR(false);
boost::this_thread::sleep(boost::posix_time::milliseconds(1000));
{
boost::lock_guard<boost::mutex> l(this->queue_mutex);
this->can_send = true;
}
}
}

View file

@ -39,7 +39,7 @@ class GCodeSender : private boost::noncopyable {
asio::io_service io;
asio::serial_port serial;
boost::thread background_thread;
boost::asio::streambuf read_buffer;
boost::asio::streambuf read_buffer, write_buffer;
bool open; // whether the serial socket is connected
bool connected; // whether the printer is online
bool error;
@ -47,11 +47,12 @@ class GCodeSender : private boost::noncopyable {
// this mutex guards queue, priqueue, can_send, queue_paused, sent, last_sent
mutable boost::mutex queue_mutex;
std::queue<std::string> queue, priqueue;
std::queue<std::string> queue;
std::list<std::string> priqueue;
bool can_send;
bool queue_paused;
size_t sent;
std::string last_sent;
std::vector<std::string> last_sent;
// this mutex guards log, T, B
mutable boost::mutex log_mutex;
@ -61,7 +62,7 @@ class GCodeSender : private boost::noncopyable {
void set_baud_rate(unsigned int baud_rate);
void set_error_status(bool e);
void do_send();
void do_send(const std::string &line);
void on_write(const boost::system::error_code& error, size_t bytes_transferred);
void do_close();
void do_read();
void on_read(const boost::system::error_code& error, size_t bytes_transferred);