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

Use of embot::app::eth::theEncoderReader in all ETH boards #399

Merged
merged 4 commits into from
Jul 27, 2023
Merged

Use of embot::app::eth::theEncoderReader in all ETH boards #399

merged 4 commits into from
Jul 27, 2023

Conversation

marcoaccame
Copy link
Contributor

@marcoaccame marcoaccame commented Jul 21, 2023

Description

This PR enables all the ETH boards to use a common interface for the the reading of the encoders used in motion control.

It ensures:

  • easy maintenance / refactoring of the code,
  • easy extension towards future more powerful sensors

That is possible because we now use a PIMPL singleton object called embot::app::eth::theEncoderReader which is derived from an interface embot::app::eth::encoder::v1::IFreader that offers functionalities as needed by today's encoders.

But it can also be derived from other interfaces to offer easy migration to encoders w/ higher resolution or to add improved diagnostics by keeping the legacy functionalities.

Details

This PR makes the singleton embot::app::eth::theEncoderReader the unique point of usage for acquisition of encoders' information in motion control on ETH boards.

It is care of the protected implementation (PIMPL) of the singleton to manage the required resources and retrieve the requested encoder values.

erDiagram
"motion control objects" ||..|| "embot::app::eth::theEncoderReader" : use
"embot::app::eth::theEncoderReader" ||..|| "PIMPL" : uses
"PIMPL" ||..|| "HW peripherals" : manages
"PIMPL" ||..|| "CAN located encoders" : manages
Loading

Figure. The object embot::app::eth::theEncoderReader and its PIMPL.

For now we use two implementations:

  • One dedicated to boards such ems, mc4plus and mc2plus which require the HAL of the stm32f407 chip. This one uses the legacy objects EOtheEncoderReader and EOappEncodersReader which can stay as they are or can be easily refactored later on.
  • One dedicated to the amc (but later on also for the amc2foc) which can use the embot::hw framework.

The two implementations

erDiagram

"PIMPL1" ||..|| "EOtheEncoderReader" : uses
"EOtheEncoderReader" ||..|| "EOappEncodersReader" : uses
"EOappEncodersReader" ||..|| "HAL" : uses
"HAL" ||..|| "SPI encoders" : manages
"HAL" ||..|| "MOTOR encoders" : manages
"HAL" ||..|| "ADC encoders" : manages
"EOappEncodersReader" ||..|| "EOtheMAIS" : manages
"EOappEncodersReader" ||..|| "EOthePOS" : manages
"EOappEncodersReader" ||..|| "EOthePSC" : manages
Loading

Figure. Implementation used by ems, mc4plus and mc2plus: all the encoders, both HAL based and CAN located

erDiagram

"PIMPL2" ||..|| "embot::hw::encoder" : uses
"embot::hw::encoder" ||..|| "SPI encoders" : manages
Loading

Figure. Implementation used by amc: only AEA encoders so far.

Evolution of the functionalities

This singleton embot::app::eth::theEncoderReader is derived from a given interface, embot::app::eth::encoder::v1::IFreader, which ensures the encoder reading functionalities as requested by today's ETH boards: ems, mc4plus, mc2plus, amc .

erDiagram

"embot::app::eth::theEncoderReader" ||..|| "embot::app::eth::encoder::v1::IFreader" : implements
"embot::app::eth::theEncoderReader" ||..|| "embot::app::eth::encoder::experimental::IFreader" : implements
"embot::app::eth::encoder::v1::IFreader" ||..|| "today's encoders" : for
"embot::app::eth::encoder::experimental::IFreader" ||..|| "advanced encoders" : for

Loading

Figure. The interfaces implemented by embot::app::eth::theEncoderReader.

The embot::app::eth::theEncoderReader can also implement other interfaces able to offer future advances services such as high resolution encoders or improved diagnostic mechanisms.

As a mere demonstration of feasibility I have added derivation from an experimental::IFreader which can offer 64 bit resolution.

namespace embot::app::eth::encoder::experimental {
    ...
    struct Value
    {   // the value w/ a possible error     
        int64_t raw {0};  // to accomodate high resolution values
        embot::app::eth::encoder::experimental::Error error {Error::NONE}; 
    };
    ...

Code Listing. Possible new high resolution value.

class theEncoderReader : public 
                         embot::app::eth::encoder::v1::IFreader,
                         embot::app::eth::encoder::experimental::IFreader
{
public:
    static theEncoderReader& getInstance();
            
    bool initialise();
   
    // v1::IFreader
    bool Verify(const Config &config, ...) override;   
    ...
    
    // experimental::IFreader
    bool read(const embot::app::eth::encoder::experimental::Target &target,
              embot::app::eth::encoder::experimental::Value &value) override; 
    ...                         

Code Listing. The theEncoderReader is derived from a v1::IFreader which give the required services but also from a experimental::IFreader which can live alongside to enrich the functionalities.

Mergeability

Tests OK on a setup dedicated setup w/ an amc, a motor and an encoder.
So, I think that we can safely merge.

Binaries

They are in here:

@marcoaccame marcoaccame marked this pull request as draft July 21, 2023 12:01
Copy link
Member

@valegagge valegagge left a comment

Choose a reason for hiding this comment

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

Good work!
and fantastic documentation!

@marcoaccame marcoaccame merged commit 5fea0ee into robotology:devel Jul 27, 2023
bool embot::app::eth::theEncoderReader::Impl::Read(uint8_t position, embot::app::eth::encoder::v1::valueInfo &primary, embot::app::eth::encoder::v1::valueInfo &secondary)
{
eOencoderreader_valueInfo_t *e1 = reinterpret_cast<eOencoderreader_valueInfo_t*>(&primary);
eOencoderreader_valueInfo_t *e2 = reinterpret_cast<eOencoderreader_valueInfo_t*>(&primary);
Copy link
Contributor

Choose a reason for hiding this comment

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

@marcoaccame I think the error related to the problem about reading the values from the state and stateExt port is here. The assignment to e1 and e2 is made using the same reference. Fixing it to &secondary fix the problem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well spotted, thanx

MSECode added a commit to MSECode/icub-firmware that referenced this pull request Jul 28, 2023
Update evaluation on final offset to be passed by JointSet to POS
service so that user set calibration5 param for calibration type 14
as it is printed when port state is read
Add fix to bug introduced by PR robotology#399:
values not updated by state and stateExt ports
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants