Skip to content

Commit

Permalink
rackmon2: Fix large timeouts observed on wedge400
Browse files Browse the repository at this point in the history
Summary:
The current write method goes in 4 steps.
1. disableRead
2. write request
3. wait for LSR assert.
4. enableRead
5. Read response
Normally, a response would arrive after #4 thus,
marking the transaction as successful.
But occationally (1/10 times) the response
arrives before #4 thus causing the device to
completely miss the response -- resulting in a timeout.

This change, replaces this with
1. Keeping read enabled throughout.
2. ignore LSR changes (Do not do #3).
This change completely eliminates timeouts on wedge400+ with
an FTDI USB device which do not need this sequencing.

But we would still need this functionality for wedge100 where we use
the native aspeed device (Pending a verification on wedge100. If it
is not needed even on that platform, that functionality can be
completely stripped.)

Test Plan:
On one SSH session, run
`modbuscmd -t 300 -x 7 b50300000001`
a thousand times and on a parallel SSH session run
`modbuscmd -t 300 -x 7 b60300000001`
a thousand times.
Each iteration ensure we get the correct response and after the stress
test ensure that the number of timeouts are moderate (<10).

Reviewed By: garnermic

fbshipit-source-id: 7846f2875cd32f402c766f5e2599bf4171ba41b3
  • Loading branch information
amithash authored and facebook-github-bot committed Apr 8, 2022
1 parent ac5bf69 commit 8e41b68
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 9 deletions.
4 changes: 2 additions & 2 deletions common/recipes-core/rackmon2/rackmon/UARTDevice.cpp
Expand Up @@ -74,7 +74,7 @@ void UARTDevice::setAttribute(bool readEnable, int baudrate) {
}
}

void UARTDevice::waitWrite() {
void AspeedRS485Device::waitWrite() {
int loops;
for (loops = 0; loops < 100; loops++) {
int lsr;
Expand All @@ -89,7 +89,7 @@ void UARTDevice::waitWrite() {
}
}

void UARTDevice::write(const uint8_t* buf, size_t len) {
void AspeedRS485Device::write(const uint8_t* buf, size_t len) {
// Write with read disabled. Hence we need to do this
// as fast as possible. So, muck around with the priorities
// to make sure we get there.
Expand Down
11 changes: 4 additions & 7 deletions common/recipes-core/rackmon2/rackmon/UARTDevice.h
Expand Up @@ -11,7 +11,6 @@ class UARTDevice : public Device {
int baudrate_ = -1;

protected:
void waitWrite() override;
virtual void setAttribute(bool readEnable, int baudrate);

void readEnable() {
Expand All @@ -37,26 +36,24 @@ class UARTDevice : public Device {
}

void open() override;
void write(const uint8_t* buf, size_t len) override;

void write(const std::vector<uint8_t>& buf) {
write(buf.data(), buf.size());
}
};

class AspeedRS485Device : public UARTDevice {
protected:
void waitWrite() override;

public:
AspeedRS485Device(const std::string& device, int baudrate)
: UARTDevice(device, baudrate) {}
void open() override;
void write(const uint8_t* buf, size_t len) override;
};

class LocalEchoUARTDevice : public UARTDevice {
public:
LocalEchoUARTDevice(const std::string& device, int baudrate)
: UARTDevice(device, baudrate) {}
void write(const uint8_t* buf, size_t len) override;
void waitWrite() override {}
};

} // namespace rackmon

0 comments on commit 8e41b68

Please sign in to comment.