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

Add preTransmission(), postTransmission() for half-duplex #44

Merged
merged 3 commits into from Sep 11, 2016

Conversation

@kintel
Copy link
Contributor

@kintel kintel commented Sep 9, 2016

This is a suggested API change to address dealing with RS-485 transceivers in half duplex mode (Issue #33).

I'm not married to this particular way of solving it, but it's tested to work on my end.

@4-20ma
Copy link
Owner

@4-20ma 4-20ma commented Sep 10, 2016

Interested in the PR. Please fix conflicts and resubmit. Thanks.

@kintel
Copy link
Contributor Author

@kintel kintel commented Sep 10, 2016

Merged with master and resolved conflicts.

Set pre-transmission callback function.
This function gets called just before a modbus message is sent over serial.
Typical usage of this callback is to enable an RS485 transcieiver's

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

Spelling: "transcieiver's"

- ...RS485 transcieiver's
+ ...RS485 transceiver's
@@ -259,6 +261,10 @@ class ModbusMaster

// idle callback function; gets called during idle time between TX and RX
void (*_idle)();
// preTransmission callback function; gets called before writing a modbus message
void (*_preTransmission)();
// postTransmission callback function; gets after a modbus message has been sent

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

- ...gets after a modbus message...
+ ...gets called after a Modbus message...
@@ -259,6 +261,10 @@ class ModbusMaster

// idle callback function; gets called during idle time between TX and RX
void (*_idle)();
// preTransmission callback function; gets called before writing a modbus message

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

- ...gets called before writing a modbus message
+ ...gets called before writing a Modbus message
/**
Set pre-transmission callback function.
This function gets called just before a modbus message is sent over serial.

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

- ...before a modbus message
+ ...before a Modbus message
/**
Set post-transmission callback function.
This function gets called after a modbus message has finished sending

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

- ...after a modbus message...
+ ...after a Modbus message...
(i.e. after all data has been physically transmitted onto the serial
bus).
Typical usage of this callback is to enable an RS485 transcieiver's

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

- ...enable an RS485 transcieiver's
+ ...enable an RS485 transceiver's
bus).
Typical usage of this callback is to enable an RS485 transcieiver's
Received Enable pin, and disable its Driver Enable pin.

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

- Received Enable pin...
+ Receiver Enable pin...
@@ -672,6 +705,7 @@ uint8_t ModbusMaster::ModbusMasterTransaction(uint8_t u8MBFunction)
while (_serial->read() != -1);

// transmit request
if (_preTransmission) _preTransmission();

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

Let's maintain consistency with the _idle() function:

if (_preTransmission)
{
  _preTransmission();
}
@@ -683,6 +717,7 @@ uint8_t ModbusMaster::ModbusMasterTransaction(uint8_t u8MBFunction)

u8ModbusADUSize = 0;
_serial->flush(); // flush transmit buffer
if (_postTransmission) _postTransmission();

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

Let's maintain consistency with the _idle() function:

if (_postTransmission)
{
  _postTransmission();
}
@@ -45,7 +45,7 @@ Set to 1 to enable debugging features within class:
- pin 5 cycles for each millisecond timeout during the Modbus response
*/
#define __MODBUSMASTER_DEBUG__ (1)

This comment has been minimized.

@4-20ma

4-20ma Sep 11, 2016
Owner

Remove extraneous space.

@4-20ma
Copy link
Owner

@4-20ma 4-20ma commented Sep 11, 2016

Thanks for the PR, though I think after I incorporated #43, it introduced new conflicts. I've marked up the PR with minor edits (spelling, capitalization, spacing, & convention). If you'll incorporate these and fix the merge conflict, I'm ready to merge.

@4-20ma
Copy link
Owner

@4-20ma 4-20ma commented Sep 11, 2016

@kintel it occurred to me that we should include a simple example showing off the half-duplex feature. Do you have a simple bare-bones example to include?

@kintel
Copy link
Contributor Author

@kintel kintel commented Sep 11, 2016

@4-20ma I agree that an example would be nice. I can clean up one of my test sketches and submit that as a separate PR.

Thanks for being thorough btw. - it really helps knowing that someone actually read the code :)

@4-20ma
Copy link
Owner

@4-20ma 4-20ma commented Sep 11, 2016

LGTM

@4-20ma 4-20ma merged commit f22723f into 4-20ma:master Sep 11, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@4-20ma
Copy link
Owner

@4-20ma 4-20ma commented Sep 11, 2016

I look forward to a simplified example that shows off how to use the half-duplex feature. Thanks for implementing. Travis CI is now implemented (compiles each example included in examples/) so it will catch the obvious build errors automatically.

@kintel kintel mentioned this pull request Sep 11, 2016
1 of 2 tasks complete
@4-20ma 4-20ma changed the title Added preTransmission() and postTransmission(). Fixes #33 Add preTransmission(), postTransmission() Sep 11, 2016
@4-20ma 4-20ma changed the title Add preTransmission(), postTransmission() Add preTransmission(), postTransmission() for half-duplex Sep 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.