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

OverSamplingSimplex Example: Transmitter code KO with NOT_ASSIGNED? #104

Closed
pacproduct opened this Issue Dec 4, 2016 · 10 comments

Comments

Projects
None yet
2 participants
@pacproduct
Contributor

pacproduct commented Dec 4, 2016

Hello :)

I could not manage to make the example Local/OverSamplingSimplex work when configuring the pin in the Transmitter as instructed:
bus.strategy.set_pins(NOT_ASSIGNED, 12);

However, configuring the pin as follows seems to work as expected:
bus.strategy.set_pins(11, 12);
The following seems to work too:
bus.strategy.set_pin(12);

I'm note sure if I'm doing something wrong or if there's a bug somewhere?

Test conditions:

  • Microcontrollers: 2x ATMega328P
  • RF : Cheap 433Mhz receiver/transmitter: 433Mhz RF modules

Note: The problem arises too when directly connecting both arduinos with a simple wire instead of using RF modules.

Regards,
PAC

@gioblu

This comment has been minimized.

Owner

gioblu commented Dec 4, 2016

Ciao @pacproduct could you please include how you phisically connect the devices?
I am in any case running a test to be sure.

@pacproduct

This comment has been minimized.

Contributor

pacproduct commented Dec 5, 2016

Sure,

Here is the schematic of both circuits (receiver being an Arduino Uno board on the left hand, transmitter being a autonomous ATMega328P with a dedicated 16Mhz resonator on the right hand):
test_rf_pjon

When using bus.strategy.set_pins(NOT_ASSIGNED, 12); (in fact, when using your Local/OverSamplingSimplex example untouched), the receiver displays that nothing gets received correctly (on the serial console).
When using bus.strategy.set_pin(12); however, the receiver does tell us that many packets get successfully received.

We also plugged an LED in place of the transmitter to double check whether the RF transmitter was the problem or not, and it confirmed that the output pin (12) of the ATMega328P was staying low when using bus.strategy.set_pins(NOT_ASSIGNED, 12);.

Thus, it seems like the code is not initiating any transmission in this latter case.

@gioblu

This comment has been minimized.

Owner

gioblu commented Dec 6, 2016

Ciao @pacproduct I will try to be as precise as you were detailing your experiment.
Thank you for your valuable support. It seems you spotted an issue:

  • Inconsistency in the example: it misses a configuration line bus.set_communication_mode(SIMPLEX);.

It is described in the documentation but got lost during development in this sketch.

Reading back the code made me consider:

  • The mode should be handled by strategies and PJON should know nothing about it.

For your work to continue unaffected you should simply call the method as suggested.

I will reference a commit moving the mode in the strategies, knowing the pins they should be able to automatically set the mode accordingly and expose a setter for user as now implemented in the PJON class.

gioblu added a commit that referenced this issue Dec 6, 2016

@gioblu gioblu added the bug label Dec 6, 2016

@pacproduct

This comment has been minimized.

Contributor

pacproduct commented Dec 6, 2016

Many thanks for your fast responses, I'll carry on testing your library that looks promising :)

My 2 cents:
I suppose strategies could guess what mode is being used when we call set_pins() indeed, as using both parameters would mean being in half-duplex, and using NOT_ASSIGNED in place of one of them would mean being in simplex mode.
But it's still required when using set_pin() so the strategy knows if the single pin should be used for a single way or a 2-way communication... ? But then how would it know which way when used in Simplex mode? I'm a bit confused. Reading your doc I guess set_pin() is not intended to be used for Simplex mode.
I'm wondering if it wouldn't be simpler to only use function set_pins(), even if it meant setting the same pin twice when using a single wire/component for transmitting and receiving, like so: set_pins(12, 12);

Maybe worth adding the bus.set_communication_mode(SIMPLEX); example to https://github.com/gioblu/PJON/blob/master/documentation/io-setup.md too, so it's clear that when using Simplex, it should be explicitly set?

Anyway keep up the good work :)

@gioblu

This comment has been minimized.

Owner

gioblu commented Dec 6, 2016

@pacproduct ciao, can handled completely by the strategy, If a pin is NOT_ASSIGNED the strategy can set SIMPLEX, for all other use cases as you pointed out i can set_pin(12) but use SoftwareBitBang in HALF_DUPLEX so, I would leave HALF_DUPLEX as default and suggest the user to call bus.strategy.set_communication_mode(SIMPLEX); if the channel is used so. I would leave set_pin present because easy and intuitive to use with a single wire PJON bus, that is also the default setup.
For now pin assignment is not checked but can be added a return in methods where is used a pin that is NOT_ASSIGNED and avoid to execute the code (this can happen only in case of user wrong use of the api).

I agree to your proposal of repeat in io-setup an example of set_communication_mode

pacproduct added a commit to pacproduct/PJON that referenced this issue Dec 6, 2016

Update io-setup doc with set_communication_mode()
As suggested in gioblu#104 it'd be a good idea to illustrate that using NOT_ASSIGNED means that we're necessarily in Simplex mode, which needs to be manually set with bus.strategy.set_communication_mode(SIMPLEX);

Adding it to the example would help new comers to PJON.
@pacproduct

This comment has been minimized.

Contributor

pacproduct commented Dec 6, 2016

Created pull request #105 for the doc improvement I had in mind.

@gioblu

This comment has been minimized.

Owner

gioblu commented Dec 6, 2016

Ciao @pacproduct I have accepted your pull, thanks again for your support

@gioblu

This comment has been minimized.

Owner

gioblu commented Dec 6, 2016

@pacproduct I have some doubt to move the setter and the mode in the strategy, because, it seems better not to call receive_response instead of calling it and inside avoid the execution...
What do you think about it?

@pacproduct

This comment has been minimized.

Contributor

pacproduct commented Dec 7, 2016

I must say I'm not sure to get what you mean...
I'm probably not familiar enough with your library yet to have an educated opinion on this to be honest.

AFAIK, the communication mode feels like it does not belong to strategies because they do not alter it, they simply use the mode that was defined previously. If that makes sense...

@gioblu

This comment has been minimized.

Owner

gioblu commented Dec 7, 2016

I agree with you, and I understand you need some time to get used to the abstraction layers of PJON.
I will close for now this issue, and leave as it is now the mode configuration.
Thank you for your support and opinion.

@gioblu gioblu closed this Dec 7, 2016

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