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

Improved diagnostics for AksIM2 encoder #341

Merged
merged 3 commits into from
Feb 22, 2023
+92 −45
Merged

Improved diagnostics for AksIM2 encoder #341

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cabb4b9
Update hal + EOappEncodersReader to manage the diagnostic for Aksim2
sgiraz Feb 16, 2023
cc29a53
AksIM2 removed unwanted forced errors
sgiraz Feb 20, 2023
1429295
Update versions for `mc2plus`, `mc4plus`, `ems4`
sgiraz Feb 21, 2023
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
3 changes: 2 additions & 1 deletion emBODY/eBcode/arch-arm/board/ems004/appl/v2/src/eoappservices/EOtheEncoderReader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ typedef enum
encreader_err_NOTCONNECTED = 15, /* this error happens when the encoder type is none or encoder is not local, for example it is connected to 2foc board */
encreader_err_AKSIM2_INVALID_DATA = 16,
encreader_err_AKSIM2_CLOSE_TO_LIMITS= 17,
encreader_err_AKSIM2_CRC_ERROR = 18
encreader_err_AKSIM2_CRC_ERROR = 18,
encreader_err_AKSIM2_GENERIC = 19

} eOencoderreader_errortype_t;

Expand Down
96 changes: 67 additions & 29 deletions emBODY/eBcode/arch-arm/embobj/plus/board/EOappEncodersReader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,9 +657,18 @@ extern eOresult_t eo_appEncReader_GetValue(EOappEncReader *p, uint8_t jomo, eOen
}
}
else
{ // we dont even have a valid reading from hal
prop.valueinfo->errortype = encreader_err_AEA_READING;
errorparam = 0xffff;
{ // we dont even have a valid reading from hal or the encoder is not properly connected to the board
prop.valueinfo->errortype = encreader_err_AKSIM2_GENERIC ;
errorparam = 0;

// notify the error (check and re-check)
eOerrmanDescriptor_t errdes = {0};
errdes.sourcedevice = eo_errman_sourcedevice_localboard;
errdes.sourceaddress = 0;
errdes.par16 = 0;
errdes.par64 = (uint64_t) (diagn.info.aksim2_status_crc) << 32;
errdes.code = eoerror_code_get(eoerror_category_HardWare, eoerror_value_HW_encoder_not_connected);
eo_errman_Error(eo_errman_GetHandle(), eo_errortype_error, NULL, NULL, &errdes);
}
} break;

Expand Down Expand Up @@ -869,7 +878,7 @@ extern eOresult_t eo_appEncReader_GetValue(EOappEncReader *p, uint8_t jomo, eOen
// in par16[1] and par64[1] we put info of the 4 secondary encoders.
// so far we use prop.valueinfo->errortype but we may use also diagn for the amo

eObool_t filldiagnostics = eo_common_byte_bitcheck(p->diagnostics.config.jomomask, jomo);
eObool_t filldiagnostics = eo_common_byte_bitcheck(p->diagnostics.config.jomomask, jomo);
if(eobool_true == filldiagnostics)
{
if(eomc_enc_amo == prop.descriptor->type)
Expand All @@ -882,11 +891,11 @@ extern eOresult_t eo_appEncReader_GetValue(EOappEncReader *p, uint8_t jomo, eOen
// select only 2 bytes: 1 from from diagn.type and one from diagn.info.value
uint64_t word = (0x0ff & diagn.type) | ((0xff & diagn.info.value) << 8);
// copy the word in correct position so that we have [word-enc3 | word-enc2 | word-enc1 | word-enc0]
p->diagnostics.par64[i] |= (word << (16*jomo));
p->diagnostics.par64[i] |= (word << (16*jomo));
}
}
else
{
{
switch(prop.valueinfo->errortype)
{
case encreader_err_NONE:
Expand All @@ -897,10 +906,14 @@ extern eOresult_t eo_appEncReader_GetValue(EOappEncReader *p, uint8_t jomo, eOen
default:
case encreader_err_GENERIC:
{ // we dont know what is happening ... we just set the flag in par16[i].
p->diagnostics.par16[i] |= (encreader_err_GENERIC<<(4*jomo)); // shift by nibbles ..
p->diagnostics.par16[i] |= (encreader_err_GENERIC<<(4*jomo)); // shift by nibbles ..
} break;

case encreader_err_AEA_READING:
case encreader_err_AKSIM2_CLOSE_TO_LIMITS:
case encreader_err_AKSIM2_CRC_ERROR:
case encreader_err_AKSIM2_INVALID_DATA:
case encreader_err_AKSIM2_GENERIC:
case encreader_err_AEA_READING:
case encreader_err_AEA_PARITY:
case encreader_err_AEA_CHIP:
case encreader_err_QENC_GENERIC:
Expand All @@ -909,25 +922,22 @@ extern eOresult_t eo_appEncReader_GetValue(EOappEncReader *p, uint8_t jomo, eOen
case encreader_err_PSC_GENERIC:
case encreader_err_POS_GENERIC:
case encreader_err_AMO_GENERIC:
case encreader_err_AKSIM2_CLOSE_TO_LIMITS:
case encreader_err_AKSIM2_CRC_ERROR:
case encreader_err_AKSIM2_INVALID_DATA:
case encreader_err_SPICHAINOF2_GENERIC:
case encreader_err_SPICHAINOF3_GENERIC:
{ // in such cases, we report the errortype and the errorparam that someone has prepared
p->diagnostics.par16[i] |= (prop.valueinfo->errortype<<(4*jomo)); // shift by nibbles ..
p->diagnostics.par64[i] &= (errorparam<<(16*jomo)); // shift by two bytes
} break;
p->diagnostics.par64[i] &= (errorparam<<(16*jomo)); // shift by two bytes
} break;
}
}
}
}

// ok, we now go to next encoder or ... we terminate the for() loop

} // for()


// now the return value. we return always OK
// now the return value. we return always OK
return eores_OK;
}

Expand Down Expand Up @@ -1332,7 +1342,7 @@ static eObool_t s_eo_appEncReader_IsValidValue_AEA(uint32_t *valueraw, eOencoder

static eObool_t s_eo_appEncReader_IsValidValue_AEA3(uint32_t *valueraw, eOencoderreader_errortype_t *error)
{
// TODO: there are no way to check the validity when using the AEA3 in SSI mode
// TODO: there is no way to check the validity when using the AEA3 in SSI mode
// if((*valueraw & 0x01) != 0x00)
// {
// *error = encreader_err_AEA_CHIP;
Expand All @@ -1343,19 +1353,47 @@ static eObool_t s_eo_appEncReader_IsValidValue_AEA3(uint32_t *valueraw, eOencode

static eObool_t s_eo_appEncReader_IsValidValue_AKSIM2(hal_spiencoder_diagnostic_t* diag, eOencoderreader_errortype_t *error)
{
switch(diag->type)
{
case hal_spiencoder_diagnostic_type_aksim2_invalid_data:
*error = encreader_err_AKSIM2_INVALID_DATA;
return eobool_false;
case hal_spiencoder_diagnostic_type_aksim2_crc_error:
*error = encreader_err_AKSIM2_CRC_ERROR;
return eobool_false;
case hal_spiencoder_diagnostic_type_aksim2_close_to_limits:
*error = encreader_err_AKSIM2_CLOSE_TO_LIMITS;
break;
default:
*error = encreader_err_NONE;
// switch(diag->type)
// {
// case hal_spiencoder_diagnostic_type_aksim2_invalid_data:
// *error = encreader_err_AKSIM2_INVALID_DATA;
// return eobool_false;
// case hal_spiencoder_diagnostic_type_aksim2_crc_error:
// *error = encreader_err_AKSIM2_CRC_ERROR;
// return eobool_false;
// case hal_spiencoder_diagnostic_type_aksim2_close_to_limits:
// *error = encreader_err_AKSIM2_CLOSE_TO_LIMITS;
// break;
// default:
// *error = encreader_err_NONE;
// }
//
// TODO: check and re-check
eOerrmanDescriptor_t errdes = {0};
errdes.sourcedevice = eo_errman_sourcedevice_localboard;
errdes.sourceaddress = 0;
errdes.par16 = 0;
errdes.par64 = (uint64_t) (diag->info.aksim2_status_crc) << 32;

// TODO: remove
diag->info.aksim2_status_crc = 0x07;
sgiraz marked this conversation as resolved.
Show resolved Hide resolved

if(0x04 == (0x04 & diag->info.aksim2_status_crc))
{
errdes.code = eoerror_code_get(eoerror_category_HardWare, eoerror_value_HW_encoder_invalid_value);
eo_errman_Error(eo_errman_GetHandle(), eo_errortype_error, NULL, NULL, &errdes);
}

if(0x02 == (0x02 & diag->info.aksim2_status_crc))
{
errdes.code = eoerror_code_get(eoerror_category_HardWare, eoerror_value_HW_encoder_close_to_limits);
eo_errman_Error(eo_errman_GetHandle(), eo_errortype_error, NULL, NULL, &errdes);
}

if(0x01 == (0x01 & diag->info.aksim2_status_crc))
{
errdes.code = eoerror_code_get(eoerror_category_HardWare, eoerror_value_HW_encoder_crc);
eo_errman_Error(eo_errman_GetHandle(), eo_errortype_error, NULL, NULL, &errdes);
}

return eobool_true;
Copy link
Member

Choose a reason for hiding this comment

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

Hi @sgiraz ,
I think that this function should return false if one or more errors occurred in order to let the above sw layers discard the value with errors. Otherwise false.

Moreover, since with this procedure, we send an explicit error message for each error event, the in/out parameter error should value encreader_err_NONE. In this way, in the block of code that takes care to send the diagnostic, it doesn't send anything despite of the value of filldiagnostics.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I noticed only now that my comment was pending...sorry!

@sgiraz we can discuss this together and do a fix. :)

Copy link
Contributor Author

@sgiraz sgiraz Feb 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

No worries @valegagge!
As discussed f2f I gonna apply these fixies in a new PR on icub-firmware and icub-firmware-build.

cc @marcoaccame

Expand Down
1 change: 1 addition & 0 deletions emBODY/eBcode/arch-arm/libs/highlevel/abslayer/hal2/api/hal_spiencoder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ typedef enum
hal_spiencoder_diagnostic_type_aksim2_invalid_data = 5,
hal_spiencoder_diagnostic_type_aksim2_close_to_limits = 6,
hal_spiencoder_diagnostic_type_aksim2_crc_error = 7,
hal_spiencoder_diagnostic_type_aksim2_not_connected = 8,
} hal_spiencoder_diagnostic_type_t;

typedef struct
Expand Down
10 changes: 10 additions & 0 deletions emBODY/eBcode/arch-arm/libs/highlevel/abslayer/hal2/src/extra/devices/hal_spiencoder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,16 @@ extern hal_result_t hal_spiencoder_get_value2(hal_spiencoder_t id, hal_spiencode
diagn->info.aksim2_status_crc |= 0x01;
}


// Check for SPI reading errors: if all data are FF then the encoder is not connected or the SPI is not working.
if (intitem->multiturncounter == 0xFFFF && intitem->position == 0x7FFFF && intitem->status_bits == 0x03 && intitem->crc == 0xFF)
{
// TODO: check if it can be manage it, or remove everything berfore the return because these diagnostic is not currently used.
diagn->type = hal_spiencoder_diagnostic_type_aksim2_not_connected;
diagn->info.value = 0;
return hal_res_NOK_generic;
}

*pos = intitem->position;
}
else if (intitem->config.type == hal_spiencoder_typeAMO)
Expand Down