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

E1.33 cherry pick third time lucky #1947

Merged
merged 33 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
05b0dbd
Add whitespace after single line code sections in readme so they wrap…
peternewman Mar 16, 2024
e85c5a5
Fix a typo
peternewman Mar 16, 2024
740de39
Merge branch 'master' of https://github.com/openlightingproject/ola i…
peternewman Mar 21, 2024
2e0d488
Initial handling of LLRPProbeRequest PDUs
peternewman Feb 6, 2020
f608d4e
Allow the RDMInflator to be used with both native E1.33 and LLRP RDM …
peternewman Feb 11, 2020
7e821e2
Add the ability to pack an RDMPDU
peternewman Feb 11, 2020
76d73f3
Add the ability to inflate an LLRP Probe Reply and build a Probe Request
peternewman Feb 18, 2020
0906483
First attempt at the E1.33 Broker PDU
peternewman Mar 7, 2023
a81c83d
First attempt at BrokerClientEntryPDU
peternewman Mar 7, 2023
cf6c53b
First go at BrokerConnectPDU, could do with more tests
peternewman Mar 7, 2023
87cb468
Add the BrokerNullPDU and tests
peternewman Mar 8, 2023
77eefe4
Correct the byte order for the E1.33 version field
peternewman Mar 9, 2023
17b625e
Correct the size of the BrokerPDU's vector
peternewman Mar 9, 2023
3ce250c
Correct a broker endian check for the E1.33 version in our tests
peternewman Mar 9, 2023
f4db0ee
Add a testPrepend test for the BrokerClientEntryPDU base class
peternewman Mar 9, 2023
b14741f
Allow the timeout interval of the HealthCheckedConnection to be custo…
peternewman Mar 10, 2023
f70cb7d
Add some more new simple E1.33 inflators
peternewman Jul 3, 2023
a44aa2c
More E1.33 vectors
peternewman Jul 3, 2023
f6710a4
Fix a minor typo
peternewman Jul 3, 2023
97c8f81
A few more E1.33 enums
peternewman Jul 3, 2023
f64d20f
Fix a lint issue
peternewman Jul 3, 2023
efe0f47
Remove some duplicate enums from a broken merge
peternewman Mar 22, 2024
6cc4d2f
Fix another broken merge
peternewman Mar 22, 2024
fd70744
Fix lint in LLRPProbeRequestPDU.cpp
peternewman Mar 22, 2024
a7a9d97
Remove some reundant using statements
peternewman Mar 22, 2024
6f26169
Fix lots of lint issues
peternewman Mar 22, 2024
684d700
Lint RDMPDU
peternewman Mar 22, 2024
db18da4
Fix a number of Broker lint issues
peternewman Mar 23, 2024
76e6181
Fix a number of LLRP lint issues
peternewman Mar 23, 2024
58e0a58
Fix RDM Inflator lint issues
peternewman Mar 23, 2024
f0b01b3
Fix more lint issues
peternewman Mar 23, 2024
baf60c9
Add a todo for future callback changes
peternewman Mar 25, 2024
a58d5e6
Fix some minor comments
peternewman Mar 25, 2024
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
17 changes: 13 additions & 4 deletions common/network/HealthCheckedConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,25 @@ namespace network {

HealthCheckedConnection::HealthCheckedConnection(
ola::thread::SchedulerInterface *scheduler,
const ola::TimeInterval heartbeat_interval,
const ola::TimeInterval timeout_interval)
: m_scheduler(scheduler),
m_heartbeat_interval(timeout_interval),
m_heartbeat_interval(heartbeat_interval),
m_timeout_interval(timeout_interval),
m_send_timeout_id(ola::thread::INVALID_TIMEOUT),
m_receive_timeout_id(ola::thread::INVALID_TIMEOUT) {
}


HealthCheckedConnection::HealthCheckedConnection(
ola::thread::SchedulerInterface *scheduler,
const ola::TimeInterval heartbeat_interval)
: HealthCheckedConnection(scheduler,
heartbeat_interval,
ola::TimeInterval(static_cast<int>(
2.5 * heartbeat_interval.AsInt()))) {
}

HealthCheckedConnection::~HealthCheckedConnection() {
if (m_send_timeout_id != ola::thread::INVALID_TIMEOUT)
m_scheduler->RemoveTimeout(m_send_timeout_id);
Expand Down Expand Up @@ -101,10 +112,8 @@ bool HealthCheckedConnection::SendNextHeartbeat() {


void HealthCheckedConnection::UpdateReceiveTimer() {
TimeInterval timeout_interval(static_cast<int>(
2.5 * m_heartbeat_interval.AsInt()));
m_receive_timeout_id = m_scheduler->RegisterSingleTimeout(
timeout_interval,
m_timeout_interval,
NewSingleCallback(
this, &HealthCheckedConnection::InternalHeartbeatTimeout));
}
Expand Down
85 changes: 82 additions & 3 deletions common/network/HealthCheckedConnectionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,28 @@ class MockHealthCheckedConnection: public HealthCheckedConnection {

MockHealthCheckedConnection(ola::io::ConnectedDescriptor *descriptor,
SelectServer *scheduler,
const ola::TimeInterval heartbeat_interval,
const ola::TimeInterval timeout_interval,
const Options &options,
MockClock *clock)
: HealthCheckedConnection(scheduler, timeout_interval),
: HealthCheckedConnection(scheduler,
heartbeat_interval,
timeout_interval),
m_descriptor(descriptor),
m_ss(scheduler),
m_options(options),
m_next_heartbeat(0),
m_expected_heartbeat(0),
m_channel_ok(true),
m_clock(clock) {
}

MockHealthCheckedConnection(ola::io::ConnectedDescriptor *descriptor,
SelectServer *scheduler,
const ola::TimeInterval heartbeat_interval,
const Options &options,
MockClock *clock)
: HealthCheckedConnection(scheduler, heartbeat_interval),
m_descriptor(descriptor),
m_ss(scheduler),
m_options(options),
Expand All @@ -70,8 +88,10 @@ class MockHealthCheckedConnection: public HealthCheckedConnection {
}

void SendHeartbeat() {
OLA_DEBUG << "Maybe send heartbeat";
if (m_options.send_every == 0 ||
m_next_heartbeat % m_options.send_every == 0) {
OLA_DEBUG << "Sending heartbeat";
m_descriptor->Send(&m_next_heartbeat, sizeof(m_next_heartbeat));
}
m_clock->AdvanceTime(0, 180000);
Expand Down Expand Up @@ -115,13 +135,18 @@ class HealthCheckedConnectionTest: public CppUnit::TestFixture {
HealthCheckedConnectionTest()
: CppUnit::TestFixture(),
m_ss(NULL, &m_clock),
heartbeat_interval(0, 200000) {
heartbeat_interval(0, 200000),
// Allow a little bit of wiggle room so we don't hit timing issues
// when running the tests
timeout_interval(0, 650000) {
}

CPPUNIT_TEST_SUITE(HealthCheckedConnectionTest);
CPPUNIT_TEST(testSimpleChannel);
CPPUNIT_TEST(testChannelWithPacketLoss);
CPPUNIT_TEST(testChannelWithHeavyPacketLoss);
CPPUNIT_TEST(testChannelWithHeavyPacketLossLongerTimeout);
CPPUNIT_TEST(testChannelWithVeryHeavyPacketLossLongerTimeout);
CPPUNIT_TEST(testPauseAndResume);
CPPUNIT_TEST_SUITE_END();

Expand All @@ -131,6 +156,8 @@ class HealthCheckedConnectionTest: public CppUnit::TestFixture {
void testSimpleChannel();
void testChannelWithPacketLoss();
void testChannelWithHeavyPacketLoss();
void testChannelWithHeavyPacketLossLongerTimeout();
void testChannelWithVeryHeavyPacketLossLongerTimeout();
void testPauseAndResume();

void PauseReading(MockHealthCheckedConnection *connection) {
Expand All @@ -148,6 +175,7 @@ class HealthCheckedConnectionTest: public CppUnit::TestFixture {
SelectServer m_ss;
LoopbackDescriptor socket;
TimeInterval heartbeat_interval;
TimeInterval timeout_interval;
MockHealthCheckedConnection::Options options;
};

Expand Down Expand Up @@ -206,7 +234,7 @@ void HealthCheckedConnectionTest::testChannelWithPacketLoss() {


/**
* Check the channel works when every 2nd heartbeat is lost
* Check the channel fails when 2 of every 3 heartbeats are lost
*/
void HealthCheckedConnectionTest::testChannelWithHeavyPacketLoss() {
options.send_every = 3;
Expand All @@ -228,6 +256,57 @@ void HealthCheckedConnectionTest::testChannelWithHeavyPacketLoss() {
}


/**
* Check the channel works when 2 of every 3 heartbeats are lost but the
* timeout interval is 3 * heartbeat_interval rather than the default
*/
void HealthCheckedConnectionTest::
testChannelWithHeavyPacketLossLongerTimeout() {
options.send_every = 3;
MockHealthCheckedConnection connection(&socket,
&m_ss,
heartbeat_interval,
timeout_interval,
options,
&m_clock);

socket.SetOnData(
NewCallback(&connection, &MockHealthCheckedConnection::ReadData));
connection.Setup();
m_ss.AddReadDescriptor(&socket);
connection.Setup();

m_ss.Run();
OLA_ASSERT_TRUE(connection.ChannelOk());
}


/**
* Check the channel fails when 3 of every 4 heartbeats are lost even though
* the timeout interval is 3 * heartbeat_interval
*/
void HealthCheckedConnectionTest::
testChannelWithVeryHeavyPacketLossLongerTimeout() {
options.send_every = 4;
options.abort_on_failure = false;
MockHealthCheckedConnection connection(&socket,
&m_ss,
heartbeat_interval,
timeout_interval,
options,
&m_clock);

socket.SetOnData(
NewCallback(&connection, &MockHealthCheckedConnection::ReadData));
connection.Setup();
m_ss.AddReadDescriptor(&socket);
connection.Setup();

m_ss.Run();
OLA_ASSERT_FALSE(connection.ChannelOk());
}


/**
* Check pausing doesn't mark the channel as bad.
*/
Expand Down
5 changes: 5 additions & 0 deletions include/ola/e133/E133Enums.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ enum E133DisconnectStatusCode {
enum {
MAX_E133_STATUS_STRING_SIZE = 64
Copy link
Member

Choose a reason for hiding this comment

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

Mh, you use single-element enums here and static const uint8_t in include/ola/rdm/RDMEnums.h (see MAX_RDM_DOMAIN_NAME_LENGTH). Is this on purpose or an inconsistency? Same for E133_VERSION below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ask @nomis52 from 11 years ago! 😆
https://github.com/OpenLightingProject/ola/blame/f0b01b359b4bfc3e24eb0a7a2aa96028638128ef/include/ola/e133/E133Enums.h#L95

I just copied it for the version, but I guess we could theoretically support a few versions at the same time so maybe they should be an enum at least?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, if multiple values make sense, keep it as an enum. How about this MAX_STRING_SIZE?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not entirely sure they do necessarily. I've kicked it down the road a bit and added a todo in #1841 anyway.

};

// The E1.33 version..
kripton marked this conversation as resolved.
Show resolved Hide resolved
enum {
E133_VERSION = 1
};
} // namespace e133
} // namespace ola
#endif // INCLUDE_OLA_E133_E133ENUMS_H_
13 changes: 9 additions & 4 deletions include/ola/network/HealthCheckedConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@
*
* This class adds health checking to a connection, which ensures that the
* connection is able to transfer data in a timely manner. The implementation
* is pretty simple: we define a heart beat interval I, which *must* be the
* is pretty simple: we define a heartbeat interval I, which *must* be the
* same at both ends of the connection. Every I seconds, both ends send a
* heart beat message and if either end doesn't receive a heart beat in
* 2.5 * I, the connection is deemed dead, and the connection is closed.
* heartbeat message and if either end doesn't receive a heartbeat within the
* timeout interval (which defaults to 2.5 * I if not specified), the
* connection is deemed dead, and the connection is closed.
*
* This class provides the basic health check mechanism, the sub class is left
* to define the format of the heartbeat message.
*
* To use this health checked channel, subclass HealthCheckedConnection, and
* provide the SendHeartbeat() and HeartbeatTimeout methods.
* provide the SendHeartbeat() and HeartbeatTimeout() methods.
*
* There are some additional features:
* - Some receivers may want to stop reading from a connection under some
Expand Down Expand Up @@ -57,7 +58,10 @@ namespace network {
class HealthCheckedConnection {
public:
HealthCheckedConnection(ola::thread::SchedulerInterface *scheduler,
const ola::TimeInterval heartbeat_interval,
const ola::TimeInterval timeout_interval);
HealthCheckedConnection(ola::thread::SchedulerInterface *scheduler,
const ola::TimeInterval heartbeat_interval);
virtual ~HealthCheckedConnection();

/**
Expand Down Expand Up @@ -106,6 +110,7 @@ class HealthCheckedConnection {
private:
ola::thread::SchedulerInterface *m_scheduler;
ola::TimeInterval m_heartbeat_interval;
ola::TimeInterval m_timeout_interval;
ola::thread::timeout_id m_send_timeout_id;
ola::thread::timeout_id m_receive_timeout_id;

Expand Down
3 changes: 3 additions & 0 deletions include/ola/rdm/RDMEnums.h
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,9 @@ static const uint8_t MAX_RDM_HOSTNAME_LENGTH = 63;
static const uint8_t MAX_RDM_DOMAIN_NAME_LENGTH = 231;

static const uint8_t DNS_NAME_SERVER_MAX_INDEX = 2;

// Excluding the mandatory NULL terminator
static const uint8_t MAX_RDM_SCOPE_STRING_LENGTH = 62;
} // namespace rdm
} // namespace ola
#endif // INCLUDE_OLA_RDM_RDMENUMS_H_
55 changes: 55 additions & 0 deletions libs/acn/BrokerClientAddInflator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Library General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*
* BrokerClientAddInflator.h
* Interface for the BrokerClientAddInflator class.
* Copyright (C) 2023 Peter Newman
*/

#ifndef LIBS_ACN_BROKERCLIENTADDINFLATOR_H_
#define LIBS_ACN_BROKERCLIENTADDINFLATOR_H_

#include "ola/acn/ACNVectors.h"
#include "libs/acn/BaseInflator.h"

namespace ola {
namespace acn {

class BrokerClientAddInflator: public BaseInflator {
friend class BrokerClientAddInflatorTest;

public:
BrokerClientAddInflator()
: BaseInflator() {
}
~BrokerClientAddInflator() {}

uint32_t Id() const { return ola::acn::VECTOR_BROKER_CLIENT_ADD; }

protected:
// The 'header' is 0 bytes in length.
bool DecodeHeader(HeaderSet*,
const uint8_t*,
unsigned int,
unsigned int *bytes_used) {
*bytes_used = 0;
return true;
}

void ResetHeaderField() {} // namespace noop
};
} // namespace acn
} // namespace ola
#endif // LIBS_ACN_BROKERCLIENTADDINFLATOR_H_
55 changes: 55 additions & 0 deletions libs/acn/BrokerClientEntryChangeInflator.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Library General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*
* BrokerClientEntryChangeInflator.h
* Interface for the BrokerClientEntryChangeInflator class.
* Copyright (C) 2023 Peter Newman
*/

#ifndef LIBS_ACN_BROKERCLIENTENTRYCHANGEINFLATOR_H_
#define LIBS_ACN_BROKERCLIENTENTRYCHANGEINFLATOR_H_

#include "ola/acn/ACNVectors.h"
#include "libs/acn/BaseInflator.h"

namespace ola {
namespace acn {

class BrokerClientEntryChangeInflator: public BaseInflator {
friend class BrokerClientEntryChangeInflatorTest;

public:
BrokerClientEntryChangeInflator()
: BaseInflator() {
}
~BrokerClientEntryChangeInflator() {}

uint32_t Id() const { return ola::acn::VECTOR_BROKER_CLIENT_ENTRY_CHANGE; }

protected:
// The 'header' is 0 bytes in length.
bool DecodeHeader(HeaderSet*,
const uint8_t*,
unsigned int,
unsigned int *bytes_used) {
*bytes_used = 0;
return true;
}

void ResetHeaderField() {} // namespace noop
};
} // namespace acn
} // namespace ola
#endif // LIBS_ACN_BROKERCLIENTENTRYCHANGEINFLATOR_H_
Loading
Loading