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

Finalize CAN rework #243

Merged
merged 126 commits into from
Jan 27, 2020
Merged

Finalize CAN rework #243

merged 126 commits into from
Jan 27, 2020

Conversation

PeterBowman
Copy link
Member

@PeterBowman PeterBowman commented Jan 8, 2020

Follow-up of #229 and a final (?) take on the [CAN-TEO] project.

Closes #160, closes #170, closes #221, closes #222, closes #223, closes #231, closes #232, closes #242.


The following substasks have been considered but finally discarded or deferred until a later moment:

  • Hide StateObserverLib in CanOpenNodeLib's implementation (mark as PRIVATE in CMake's usage requirements).
  • Refactor ICanBusSharer::registerSender into its own interface (similarly to CanMessageNotifier), use this in CanOpenNodeLib's classes.
  • Rename CanMessageNotifier to ICanMessageNotifier?
  • Investigate timeouts in id25 related to the transition from Switched on to Operation enabled (ref).
  • Decide on the correct/expected behavior of Technosoft's monitor thread (ref). Should it always attempt reconnection after a heartbeat failure?
  • There is something wrong with left arm CAN bus, we suspect some kind of hardware issue.
  • Review debug logs in TechnosoftIpos, print CAN ID.
  • Improve doxygen.

There is little advantage in using this CAN feature and may affect
performance.
@coveralls
Copy link

coveralls commented Jan 8, 2020

Coverage Status

Coverage increased (+0.05%) to 99.218% when pulling ef71fc0 on road-to-canhalla into 71c5524 on develop.

In case a string literal was passed, compilers kept choosing the
template version instead.
@PeterBowman PeterBowman marked this pull request as ready for review January 27, 2020 18:21
Caused by monitor threads still attempting to perform a SDO transfer
after CAN bus threads have been stopped.
This exploded on TEO's rightArm-lacquey configuration with a simple
`getEncoders` call as Lacquey does not implement IEncoders. Since this
mapper targets all available devices, it needs to have this restriction
relaxed and hope callers will sort it out on their end (e.g. cbw2).
@PeterBowman
Copy link
Member Author

Now it is good to go. Optional side-quests have been added in the description.

Keep in mind commit ef71fc0 concerning the internal template implementation check (ok boolean) prior to returning from the following functions:

//! Full-joint command mapping. See class description.
template<typename T, typename... T_refs>
bool mapAllJoints(full_mapping_fn<T, T_refs...> fn, T_refs *... refs)
{
auto task = createTask();
bool ok = false;
for (const auto & t : getDevicesWithOffsets())
{
T * p = std::get<0>(t)->getHandle<T>();
ok |= p && (task->add(p, fn, refs + std::get<1>(t)...), true);
}
// at least one targeted device must implement the 'T' iface
return ok && task->dispatch();
}

//! Joint-group command mapping. See class description.
template<typename T, typename... T_refs>
bool mapJointGroup(multi_mapping_fn<T, T_refs...> fn, int n_joint, const int * joints, T_refs *... refs)
{
auto task = createTask();
auto devices = getDevices(n_joint, joints); // extend lifetime of local joint vector
bool ok = true;
for (const auto & t : devices)
{
T * p = std::get<0>(t)->getHandle<T>();
ok &= p && (task->add(p, fn, std::get<1>(t).size(), std::get<1>(t).data(), refs + std::get<2>(t)...), true);
}
// all targeted devices must implement the 'T' iface
return ok && task->dispatch();
}

@PeterBowman PeterBowman merged commit ec41902 into develop Jan 27, 2020
@PeterBowman PeterBowman deleted the road-to-canhalla branch January 27, 2020 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment