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

Fix carriage detection on the right #205

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix carriage detection on the right #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aad3cc5
encoders: fix carriage detection on the right
jonathanperret Oct 13, 2024
79cd0eb
Consider machine ready as soon as GC is seen
jonathanperret Nov 15, 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
91 changes: 54 additions & 37 deletions src/ayab/encoders.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,31 +150,19 @@ void Encoders::encA_rising() {
(hallValue > FILTER_L_MAX[static_cast<uint8_t>(m_machineType)]))) {
m_hallActive = Direction_t::Left;

// The garter carriage has a second set of magnets that are going to
// pass the sensor and will reset state incorrectly if allowed to
// continue.
if (Carriage_t::Garter == m_carriage) {
return;
}

// Only set the belt shift the first time a magnet passes the turn mark.
// Headed to the right.
if (!m_passedLeft && Direction_t::Right == m_direction) {
// Belt shift signal only decided in front of hall sensor
m_beltShift = digitalRead(ENC_PIN_C) != 0 ? BeltShift::Shifted : BeltShift::Regular;
m_passedLeft = true;

if (Carriage_t::Garter == m_carriage) {
// This has to be the first magnet and the belt shift needs to be swapped
// But only for the G-carriage
if (m_position < 30) {
if (BeltShift::Regular == m_beltShift) {
m_beltShift = BeltShift::Shifted;
} else {
m_beltShift = BeltShift::Regular;
}
}
}
}

// The garter carriage has a second set of magnets that are going to
// pass the sensor and will reset state incorrectly if allowed to
// continue.
if (Carriage_t::Garter == m_carriage) {
return;
}

// If the carriage is already set, ignore the rest.
Expand All @@ -200,7 +188,9 @@ void Encoders::encA_rising() {
}
} else if (m_carriage == Carriage_t::NoCarriage) {
m_carriage = detected_carriage;
} else if (m_carriage != detected_carriage && m_position > start_position) {
} else if (m_carriage == Carriage_t::Lace &&
detected_carriage == Carriage_t::Knit &&
m_position > start_position) {
m_carriage = Carriage_t::Garter;

// We swap the belt shift for the g-carriage because the point of work for
Expand Down Expand Up @@ -248,19 +238,25 @@ void Encoders::encA_falling() {
}
}

// No attempt to support KH270 on the right for now as I can't test it
if (m_machineType == Machine_t::Kh270) {
return;
}

// In front of Right Hall Sensor?
uint16_t hallValue = analogRead(EOL_PIN_R);

// Avoid 'comparison of unsigned expression < 0 is always false'
// by being explicit about that behaviour being expected.
bool hallValueSmall = false;

hallValueSmall = (hallValue < FILTER_R_MIN[static_cast<uint8_t>(m_machineType)]);

if (hallValueSmall || hallValue > FILTER_R_MAX[static_cast<uint8_t>(m_machineType)]) {
if ((hallValue < FILTER_R_MIN[static_cast<uint8_t>(m_machineType)]) ||
(hallValue > FILTER_R_MAX[static_cast<uint8_t>(m_machineType)])) {
m_hallActive = Direction_t::Right;

// Only set the belt shift when the first magnet passes the turn mark.
// The garter carriage has a second set of magnets that are going to
// pass the sensor and will reset state incorrectly if allowed to
// continue.
if (Carriage_t::Garter == m_carriage) {
return;
}

// Only set the belt shift the first time a magnet passes the turn mark.
// Headed to the left.
if (!m_passedRight && Direction_t::Left == m_direction) {
// Belt shift signal only decided in front of hall sensor
Expand All @@ -270,18 +266,39 @@ void Encoders::encA_falling() {
// Shift doens't need to be swapped for the g-carriage in this direction.
}

// The garter carriage has extra magnets that are going to
// pass the sensor and will reset state incorrectly if allowed to
// continue.
if (m_carriage == Carriage_t::Garter) {
return;
Carriage detected_carriage = Carriage_t::NoCarriage;
uint8_t start_position = END_RIGHT_MINUS_OFFSET[static_cast<uint8_t>(m_machineType)];

if (m_machineType == MachineType::Kh910) {
// Due to an error in wiring on the shield, the sensor only triggers for the K carriage,
// and with a low voltage so we can't use the same logic as for other machines.
detected_carriage = Carriage_t::Knit;
} else {
if (hallValue >= FILTER_R_MIN[static_cast<uint8_t>(m_machineType)]) {
detected_carriage = Carriage_t::Knit;
} else {
detected_carriage = Carriage_t::Lace;
}
}

if (hallValueSmall) {
m_carriage = Carriage_t::Knit;
if (m_carriage == Carriage_t::Lace &&
detected_carriage == Carriage_t::Knit &&
m_position < start_position) {
m_carriage = Carriage_t::Garter;

// Belt shift and start position were set when the first magnet passed
// the sensor and we assumed we were working with a standard carriage.

// Because we detected the leftmost magnet on the G-carriage first,
// for consistency with the left side where we detect the rightmost magnet
// first, we need to adjust the carriage position.
m_position += GARTER_L_MAGNET_SPACING;
return;
} else {
m_carriage = detected_carriage;
}

// Known position of the carriage -> overwrite position
m_position = END_RIGHT_MINUS_OFFSET[static_cast<uint8_t>(m_machineType)];
m_position = start_position;
}
}
6 changes: 5 additions & 1 deletion src/ayab/encoders.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ constexpr uint8_t ALL_MAGNETS_CLEARED_RIGHT[NUM_MACHINES] = {199U, 199U, 130U};
// If we didn't have it, we'd decide which carriage we had when the first magnet passed the sensor.
// For the garter carriage we need to see both magnets.
constexpr uint8_t GARTER_SLOP = 2U;
// Spacing between a garter carriage's outer (L-carriage-like) magnets.
// For consistency between a garter carriage starting on the left or the right,
// we need to adjust the position by this distance when starting from the right.
constexpr uint8_t GARTER_L_MAGNET_SPACING = 24U;

constexpr uint8_t START_OFFSET[NUM_MACHINES][NUM_DIRECTIONS][NUM_CARRIAGES] = {
// KH910
Expand Down Expand Up @@ -108,7 +112,7 @@ constexpr uint8_t START_OFFSET[NUM_MACHINES][NUM_DIRECTIONS][NUM_CARRIAGES] = {
// KH910 KH930 KH270
constexpr uint16_t FILTER_L_MIN[NUM_MACHINES] = { 200U, 200U, 200U};
constexpr uint16_t FILTER_L_MAX[NUM_MACHINES] = { 600U, 600U, 600U};
constexpr uint16_t FILTER_R_MIN[NUM_MACHINES] = { 200U, 0U, 0U};
constexpr uint16_t FILTER_R_MIN[NUM_MACHINES] = { 200U, 200U, 200U};
constexpr uint16_t FILTER_R_MAX[NUM_MACHINES] = {1023U, 600U, 600U};

constexpr uint16_t SOLENOIDS_BITMASK = 0xFFFFU;
Expand Down
11 changes: 7 additions & 4 deletions src/ayab/knitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ void Knitter::init() {
m_workedOnLine = false;
m_lastHall = Direction_t::NoDirection;
m_position = 0U;
m_carriage = Carriage_t::NoCarriage;
m_hallActive = Direction_t::NoDirection;
m_pixelToSet = 0;
#ifdef DBG_NOMACHINE
Expand Down Expand Up @@ -216,10 +217,12 @@ bool Knitter::isReady() {
(m_position > (END_LEFT_PLUS_OFFSET[static_cast<uint8_t>(m_machineType)] + GARTER_SLOP));
bool passedRight = (Direction_t::Left == m_direction) && (Direction_t::Right == m_lastHall) &&
(m_position < (END_RIGHT_MINUS_OFFSET[static_cast<uint8_t>(m_machineType)] - GARTER_SLOP));
// Machine is initialized when left Hall sensor is passed in Right direction
// New feature (August 2020): the machine is also initialized
// when the right Hall sensor is passed in Left direction.
if (passedLeft || passedRight) {
// Machine is initialized when the left Hall sensor is passed in Right
// direction, or the right Hall sensor is passed in Left direction. Or, as
// soon as we have detected a Garter carriage, because in that case we may
// need to start setting solenoids before the carriage center has crossed the
// turn mark.
if (passedLeft || passedRight || m_carriage == Carriage_t::Garter) {

#endif // DBG_NOMACHINE
GlobalSolenoids::setSolenoids(SOLENOIDS_BITMASK);
Expand Down
24 changes: 16 additions & 8 deletions test/test_encoders.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,30 +131,34 @@ TEST_F(EncodersTest, test_encA_rising_in_front_G_carriage) {
// Create a rising edge
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A)).WillOnce(Return(true));
// Enter rising function, direction is right
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B)).WillOnce(Return(true)).WillOnce(Return(false));
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B)).WillOnce(Return(true));
// In front of Left Hall Sensor
EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_L))
.WillOnce(Return(FILTER_L_MAX[static_cast<int8_t>(encoders->getMachineType())] + 1));
.WillOnce(Return(FILTER_L_MIN[static_cast<int8_t>(encoders->getMachineType())] - 1));
// BeltShift is regular
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_C)).WillOnce(Return(true));

encoders->encA_interrupt();

ASSERT_EQ(encoders->getCarriage(), Carriage_t::Knit);
ASSERT_EQ(encoders->getCarriage(), Carriage_t::Lace);

// Create a falling edge
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A)).WillOnce(Return(false));
// Enter falling function, direction is right
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B)).WillOnce(Return(false));
// Not in front of Right Hall sensor
EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_R))
.WillOnce(Return(FILTER_R_MAX[static_cast<int8_t>(encoders->getMachineType())] + 1));
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_C));
.WillOnce(Return(FILTER_R_MAX[static_cast<int8_t>(encoders->getMachineType())] - 1));

encoders->encA_interrupt();

// Create a rising edge
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A)).WillOnce(Return(true));
// Enter rising function, direction is right
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B)).WillOnce(Return(true));
// In front of Left Hall Sensor
EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_L))
.WillOnce(Return(FILTER_L_MIN[static_cast<int8_t>(encoders->getMachineType())] - 1));
.WillOnce(Return(FILTER_R_MAX[static_cast<int8_t>(encoders->getMachineType())] + 1));

encoders->encA_interrupt();

Expand Down Expand Up @@ -185,6 +189,8 @@ TEST_F(EncodersTest, test_encA_falling_not_in_front) {
}

TEST_F(EncodersTest, test_encA_falling_in_front) {
encoders->init(Machine_t::Kh930);
ASSERT_TRUE(encoders->getMachineType() == Machine_t::Kh930);
// Create a falling edge
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A))
.WillOnce(Return(true))
Expand All @@ -209,17 +215,19 @@ TEST_F(EncodersTest, test_encA_falling_in_front) {
ASSERT_EQ(encoders->getDirection(), Direction_t::Right);
ASSERT_EQ(encoders->getHallActive(), Direction_t::Right);
ASSERT_EQ(encoders->getPosition(), 227);
ASSERT_EQ(encoders->getCarriage(), Carriage_t::NoCarriage);
ASSERT_EQ(encoders->getCarriage(), Carriage_t::Knit);
}

// requires FILTER_R_MIN != 0
TEST_F(EncodersTest, test_encA_falling_set_K_carriage_KH910) {
encoders->init(Machine_t::Kh910);
ASSERT_TRUE(encoders->getMachineType() == Machine_t::Kh910);

// Create a rising edge
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_A)).WillOnce(Return(true));
EXPECT_CALL(*arduinoMock, digitalRead(ENC_PIN_B));
EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_L));
EXPECT_CALL(*arduinoMock, analogRead(EOL_PIN_L))
.WillOnce(Return(FILTER_R_MIN[static_cast<int8_t>(encoders->getMachineType())] + 1));
encoders->encA_interrupt();

// falling edge
Expand Down
Loading