Skip to content

FWT-146 Solve Dat One (SDO) Issue#112

Merged
DiegoLHendrix merged 51 commits into
mainfrom
feature/diegolhendrix/sdo-sample
Apr 23, 2026
Merged

FWT-146 Solve Dat One (SDO) Issue#112
DiegoLHendrix merged 51 commits into
mainfrom
feature/diegolhendrix/sdo-sample

Conversation

@DiegoLHendrix

Copy link
Copy Markdown
Contributor

No description provided.

@DiegoLHendrix DiegoLHendrix changed the title Start Drinking On (SDO) company time FWT-146 Start Drinking On (SDO) company time Dec 5, 2024

@mjh9585 mjh9585 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a few changes, mostly should make a method to register a callback that is used by the sdo.

Comment thread samples/canopen/canopen_sdo/main.cpp Outdated
Comment thread samples/canopen/canopen_tpdo/main.cpp Outdated
Comment thread samples/canopen/canopen_tpdo/main.cpp Outdated
Comment thread src/core/io/CANopen.cpp Outdated
Comment thread samples/canopen/canopen_sdo/SDOCanNode.cpp Outdated
Comment thread samples/canopen/canopen_sdo/SDOCanNode.cpp Outdated
Comment thread include/core/io/CANopen.hpp Outdated

@mjmagee991 mjmagee991 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A good start

Comment thread include/core/io/CANOpenMacros.hpp
Comment thread include/core/io/CANopen.hpp Outdated
Comment thread include/core/io/CANopen.hpp Outdated
Comment thread include/core/io/CANopen.hpp Outdated
Comment thread include/core/io/CANopen.hpp Outdated
Comment thread src/core/io/CANopen.cpp Outdated
Comment thread samples/canopen/canopen_sdo/SDOCanNode.hpp Outdated
Comment thread samples/canopen/canopen_sdo/main.cpp Outdated
Comment thread samples/canopen/canopen_sdo/main.cpp Outdated
Comment thread src/core/io/CANopen.cpp Outdated

@mjh9585 mjh9585 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a couple of changes. Make sure to clean up leftover code from the unresolved comments.

Comment thread samples/canopen/canopen_rpdo/main.cpp Outdated
Comment thread src/core/io/CANopen.cpp Outdated
Comment thread src/core/io/CANopen.cpp Outdated
@DiegoLHendrix DiegoLHendrix requested a review from mjh9585 March 17, 2026 19:31
@mjh9585 mjh9585 requested a review from aclowmclaughlin March 29, 2026 15:42

@mjh9585 mjh9585 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall pretty good, just needs a few changes and a little polish. Also you never take advantage of your blocking methods in your sample which could be helpful to see an example for simple use cases.

Comment thread samples/canopen/canopen_sdo/SDOCanNode.cpp Outdated
Comment thread include/core/io/CANopen.hpp
Comment thread include/core/io/CANopen.hpp
Comment thread include/core/io/CANopen.hpp
Comment thread include/core/io/CANopen.hpp
Comment thread src/core/io/CANopen.cpp Outdated
Comment thread src/core/io/CANopen.cpp
Comment thread src/core/io/CANopen.cpp Outdated
Comment thread src/core/io/CANopen.cpp Outdated
Comment thread src/core/io/CANopen.cpp Outdated

@aclowmclaughlin aclowmclaughlin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Minor stuff and obviously you're going to need to test it.
Approving, pending a demonstrated test setup working.

Comment thread samples/canopen/canopen_sdo/SDOCanNode.cpp Outdated
Comment thread samples/canopen/canopen_sdo/SDOCanNode.hpp

@mjh9585 mjh9585 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just need some formatting fixes,

Comment thread include/core/io/CANopen.hpp Outdated
Comment thread include/core/io/CANopen.hpp Outdated
Comment thread include/core/io/CANopen.hpp Outdated
Comment thread include/core/io/CANopen.hpp Outdated
Comment thread include/core/io/CANopen.hpp
Comment thread samples/canopen/canopen_rpdo/main.cpp
Comment thread samples/canopen/canopen_sdo/SDOCanNode.cpp
Comment thread samples/canopen/canopen_sdo/main.cpp Outdated
Comment thread samples/canopen/canopen_tpdo/main.cpp
Comment thread src/core/io/CANopen.cpp Outdated
@DiegoLHendrix DiegoLHendrix changed the title FWT-146 Start Drinking On (SDO) company time FWT-146 Start Dat One (SDO) Issue Apr 22, 2026
@DiegoLHendrix DiegoLHendrix changed the title FWT-146 Start Dat One (SDO) Issue FWT-146 Solve Dat One (SDO) Issue Apr 22, 2026
@DiegoLHendrix DiegoLHendrix merged commit c599d7f into main Apr 23, 2026
2 checks passed
@DiegoLHendrix DiegoLHendrix deleted the feature/diegolhendrix/sdo-sample branch April 23, 2026 23:21
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.

4 participants