PJON based wired transport layer

[4ib3r]

I wrote transport based on PJON library, it can operate without any additional hardware (simple wire from pin to pin). External PJON library is required to use this.

[mfalkvidd]

Nice work!

To keep consistency with the rest of the library, helping users understand how to use the MySensors features, it would be nice if the debug flag (PJON_DEBUG) could be renamed to MY_DEBUG_VERBOSE_PJON (similar to the other transports, for example RF24)
It would also be nice if the same debug print function was used, to keep output format consistent (easier for users to follow, and possible to decode using the log parser) and so that debug is printed on the correct device if redirected. See

MySensors/hal/transport/RFM69/driver/new/RFM69_new.cpp

Line 38 in 8e8f44b

#if defined(MY_DEBUG_VERBOSE_RFM69)

for how other transports do debug prints.

[4ib3r]

I added MY_DEBUG_VERBOSE_PJON in a similar way.

[mfalkvidd]

MySensors is designed to work without additional dependencies (install the MySensors library through the Arduino IDE Library Manager and all features and examples will work without additional libraries).

I see you removed the PJON library in an earlier commit. What was your reasoning behind the removal? Would it be a bad idea to include the library in MySensors (except that we'd need to keep it updated, just like we have to do with the rest of the drivers)?

[mfalkvidd]

This comment is not directly related to this pull request, but I am adding it here for reference.

To make the PJON transport easy to use for MySensors users, we should create a page similar to https://www.mysensors.org/build/rs485
We could probably link to https://github.com/gioblu/PJON/tree/master/src/strategies/SoftwareBitBang which already has detailed information on which pins can be used on different microcontrollers.

The name of the page could be something like Wired (PJON). Not sure if we should plan ahead for the other PJON "strategies" (which includes AnalogRead which uses visible light or IR so not wired).

[4ib3r]

PJON library has include directive with <>, when I simple copied library to driver directory, I got compilation errors. The simplest method to resolve this problem is remove library, and use it as external dependency.

[gioblu]

Ciao @mfalkvidd and @4ib3r I see the <> may make the integration more complex, we are testing to see if we can switch to "" for internal inclusions without breaking things.

If help is required I am available.
Thank you for MySensors really great library.

[gioblu]

Ciao @4ib3r I have pushed the inclusion change see: gioblu/PJON@6fcfd75

[4ib3r]

Thank you for your change @gioblu.

[gioblu]

Ciao @4ib3r can I help you in any way?

[tekka007]

Closed due to inactivity

[gryzli133]

is there a way to speed up the PJON integration in MySensors?

[gioblu]

I would also be happy to contribute @tekka007 to make this functional and part of the next release.

[tekka007]

Ok, I'll create a follow-up PR that includes the work done in this PR and the required changes for MySensors CI. Since I have no experience with PJON and limited bandwith for testing and troubleshooting, I'd appreciate if @4ib3r, @gryzli133 and @gioblu could step in for Q&As.

[gioblu]

Ciao @tekka007 thank you very much.
I think could have sense to include the PJON implementation version 11.2: https://github.com/gioblu/PJON/releases
It is expected to be fully backward compatible so it can be substituted without pain.

Another thing may have sense to be changed is the name of the transport, being expressly dedicated to the SoftwareBitBang strategy it may have sense to say it in the name, so something like MyTransportPJON-SoftwareBitBang specially because other transports could be created leveraging of other strategies, like for example MyTransportPJON-ESPNOW.

As far as it seems it is probably unfeasible to try to make a single transport configurable to use any supported strategy.

The code seems fine, I see here:

MySensors/hal/transport/PJON/MyTransportPJON.cpp

Line 108 in 29016cd

bool transportSanityCheck(void)

transportSanityCheck is not true that is not implemented, it is already done by PJON on a lower level, if the packet is received it means it passed the 2 crcs and the packet "sanity check" of PJON.