Skip to content

libgps needs update to disable Tx pin #163

@PropGit

Description

@PropGit
Contributor

libgps is using pst.spin's full-duplex serial code, and is currently always reading from an Rx pin and setting a Tx pin to an output.

We need to have it either always disable the Tx pin (ie: leave that pin as an input), or conditionally disable it if the given pin is a certain value (ie: 32).

The choice depends on whether we only need to receive (Rx) from GPSes.

The pst.spin serial object's PASM code (used in the cog to perform full-duplex serial) always sets the Tx pin to an output, UNLESS you specify mode 0b0100 (open drain/source tx), in which case it will leave it as an input (which is what we want).

Problem is, gps_run.c always sets the mode to 0:
gps_ser = fdserial_open(_gps_rx_pin, _gps_tx_pin, 0, _gps_baud);

I'm not sure if we ever need to have receive and transmit for any current of future GPS, but the solution would be to update gps_run.c to either

  • always set mode to 0b0100, or
  • set mode to 0 except when _gps_tx_pin is the special-value 32, in which case mode is set to 0b0100

Activity

MatzElectronics

MatzElectronics commented on Dec 2, 2017

@MatzElectronics
Contributor

I wonder if an even more appropriate fix is for fd_serial itself shut off the RX or TX when an out-of-range is used...

AndyLindsay

AndyLindsay commented on Dec 4, 2017

@AndyLindsay
Contributor

Rx-only mode test code added to Dev branch with Matt's approach.

Details here.

Simple Libraries folder for testing C code with txpin = 32 in SimpleIDE is here.

PropGit

PropGit commented on Dec 4, 2017

@PropGit
ContributorAuthor

@MatzElectronics @AndyLindsay

Are we setting mode to FDSERIAL_MODE_RX_ONLY in C code somewhere, and when fdserial gets ahold of it, it clears that bit and ensures FDSERIAL_MODE_OPENDRAIN_TX is set? If so, this this (below) makes sense to me, but run_gps.c still sets mode to 0.

As-written, the PST object's mode is a 4-bit value (only 3-bits of which are used by the PASM code), but FDSERIAL_MODE_RX_ONLY assumes a 5-bit value. Also when mode is set to FDSERIAL_MODE_OPENDRAIN_TX, the PST object does not set the txpin to output, effectively making the object work with RX only.

This line fdptr->mode &= (~FDSERIAL_MODE_RX_ONLY); just keeps fdptr->mode set to the same bit pattern as before, without BIT 4 (the bit that's not used by the PST object).

I can see that the C code sets an rxOnly flag also, and appears to not bother transmitting if transmission was asked for when rsOnly is set. That's great.

AndyLindsay

AndyLindsay commented on Dec 4, 2017

@AndyLindsay
Contributor

That sums it up nicely, just one thing to add. User C code can either set the FDSERIAL_MODE_RX_ONLY mode bit in fdserial_open(rx, tx, mode, baud) or select a tx I/O pin value outside 0...31. In the case of the gps library, a tx pin argument of -1 (or Matt used 32) will cause fdserial_open to respond the same way that it would to the FDSERIAL_MODE_RX_ONLY mode bit.

A test to find out if it solves the problem will happen either Wednesday or Thursday.

The gps library could still use a variation on its current open function to allow the user to set custom modes for their GPS applications.

I added a variable to the fdserial_st type (a structure with tx, rx, mode, baud, and pointer to a buffer) to keep track of whether FDSERIAL_MODE_RX_ONLY was set. However, if the Parallax Serial Terminal PASM completely ignores the higher mode bits, I'd prefer to remove that variable from fdserial_st and just use the FDSERIAL_MODE_RX_ONLY bit. Its only job would be to prevent fdserial_tx from attempting to send data if it is called in a receive-only application.

If the Parallax Serial Terminal PASM gets confused by higher mode bits, the rfidser library may need to be changed. Reason being, rfidser also has a variable appended to its version of the fdserial_st structure, but it is used for a different purpose.

PropGit

PropGit commented on Dec 5, 2017

@PropGit
ContributorAuthor

@AndyLindsay - Follow-up on "... ignores the higher mode bits..."

Yes, Parallax Serial Terminal PASM completely ignores all bits except 0, 1, and 2; checking just for the states of those bits using a test instruction against %001, %010, and %101, respectively. The Spin code (which isn't used in PropGCC, of course) ignores all bits except bit 3, checking its value by ANDing %1000.

So you can remove that extra flag if you want and just keep the mode as set by FDSERIAL_MODE_RX_ONLY; however, if PST is ever updated to include the use of higher bits, things won't work right unless it skips those bits that have dependencies in outside C code. Not the most ideal situation, but sometimes these dependencies are not practical to code around.

I think it's fine to remove that extra code and variable from that structure.

added a commit that references this issue on Dec 18, 2017
AndyLindsay

AndyLindsay commented on Dec 18, 2017

@AndyLindsay
Contributor

libfdserial updated to use bit 4 of fdserial_st mode variable. Bug symptoms reproduced with Stock SimpleIDE v1.1 libraries, and corrected with this update to the demo branch.

AndyLindsay

AndyLindsay commented on Dec 18, 2017

@AndyLindsay
Contributor
/*
  Test fdserial library rx-only mode changes.c
  
  With fdserial library correction:
  Displays hello followed by goodbye after a 1 second pause
  
  With Simple Libraries from SimpleIDE v1.1
  Displays a white square where hello should appear.
*/  

#include "simpletools.h"
#include "gps.h"
#include "oledc.h"
#include "colormath.h"

int main()
{
  oledc_init(0, 1, 2, 3, 4, 2);
  oledc_setTextSize(SMALL);
  oledc_setTextFont(FONT_SERIF);
  oledc_setTextColor(oledc_color565(get8bitColor(0xFFFFFF, "RED"), get8bitColor(0xFFFFFF, "GREEN"), get8bitColor(0xFFFFFF, "BLUE")), oledc_color565(get8bitColor(0x000000, "RED"), get8bitColor(0x000000, "GREEN"), get8bitColor(0x000000, "BLUE")));
  pause(100);
  oledc_setCursor(0, 0, 0);
  oledc_drawText("Hello");
  pause(1000);
  oledc_setCursor(0, 20, 0);
  gps_open(14, 32, 9600);
  pause(100);
  oledc_drawText("Goodbye");
  
  while(1);
}

AndyLindsay

AndyLindsay commented on Dec 18, 2017

@AndyLindsay
Contributor

@PropGit

Thank you for verifying that bit 4 of fdserial_st mode should have no effect on the PASM; that really helped. I think this approach will make this bug fix easier to maintain and less likely to introduce bugs elsewhere.

If PST is updated to use higher bits, hopefully it'll be possible to reserve bit 4 for rx-only.

Thanks again!

Andy

PropGit

PropGit commented on Dec 18, 2017

@PropGit
ContributorAuthor

@AndyLindsay - You're welcome!

AndyLindsay

AndyLindsay commented on Dec 20, 2017

@AndyLindsay
Contributor

A logic error was corrected that prevented tx in non-rx-only mode. Corrected in demo - Correct logic mistake in Test fdserial rx only mode and dev - Correct logic mistake in rx only mode bug fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @PropGit@AndyLindsay@MatzElectronics

        Issue actions

          libgps needs update to disable Tx pin · Issue #163 · parallaxinc/Simple-Libraries