Move SPI freq to config.h

[frankleonrose]

10MHz as a default is too high, IMO. Better to start with a lower frequency that will work with more Arduino boards and SPI peripherals that might be on same SPI circuit.

[frankleonrose]

Shall I instead move the setting into config.h?

[Oliv4945]

Hi,

I discoverded that the Uno r3 is not able to reach such high speeds in #9 too, and I have no opinion about the default frequency, but moving it to config.h is definitely a good idea !

[hallard]

RFM95 datasheet says SPI frequency MAX is 10MHz, so it should be okay every time, but playing with limits is not always a good advice, to be sure I would set it to 8MHz by default.
just my 2 cents

[frankleonrose]

Thanks, @hallard. It's not necessarily a limit of the RFM95 I'm concerned about. I think we want to pick a number that increases the chance that someone using the library will have success when they first use it. That's why I would choose 1MHz - it's more likely to work with other SPI peripherals on the same bus and with older Arduino boards.

People who need to optimize communication (8MHz vs 1MHz) will look for the controls and change them. For them, it's good to bring the value into config.h to make it clear that it is a configurable parameter of the system.

In short, we should prefer requiring someone who cares about speed to push the number up rather than requiring someone who's just trying to get a board working to spend time debugging a clock speed that is too high.

[hallard]

good point @frankleonrose ;-)

by the way I never saw SPI device limited to 1MHz, le lowest I saw was 4MHz, for 1Mhz provider may need to get back to I2C ;-)

BTW, I'm not sure on the field that users will know it's "just" working at 1Mhz and it's possible to increase speed in code. May be the solution would be to bring a method and call this method into example (or comment it to be sure) so users will see directly and if not called it will works by default at 1MHz

  // LMIC init
  os_init();

  // Uncomment this line to set SPI speed to 8MHz
  // LMIC_setSPISpeed(8000000);

  // Reset the MAC state. Session and pending data transfers will be discarded.
  LMIC_reset();

[frankleonrose]

I know @matthijskooijman is leery of adding to the LMIC API, so I'm not going to over-implement this one minor setting. I think having it in the config.h with annotation is sufficient.

[matthijskooijman]

BTW, I'm not sure on the field that users will know it's "just" working at 1Mhz and it's possible to increase speed in code. May be the solution would be to bring a method and call this method into example (or comment it to be sure) so users will see directly and if not called it will works by default at 1MHz

I think this would actually be useful, since it allows setting the speed in the example sketches and exposes this possibility to users. I'm not worried about adding extra API for this, since this API would be very much Arduino-specific, and it would not affect the core of LMIC in any way. Having an API called LMIC_setSPISpeed would then also make sense, since that seems to me the only part of the SPISettings used that you would want to customize (the other parts are MSB/LSB first and clock polarity, which are fixed by the SX chip used).

If either of you would care to implement this, I'd be happy to merge that change (note that I'm a bit busy atm, but I'm certainly planning to merge the PR's that have been submitted within the next few weeks).

I know @matthijskooijman is leery of adding to the LMIC API, so I'm not going to over-implement this one minor setting. I think having it in the config.h with annotation is sufficient.

I'm mostly hesitant to divert too much from "upstream" LMIC as released by IBM. However, I've since learned that IBM will no longer work on LMIC, and there is some talk of moving upstream (e.g. non-Arduino specific) maintainance of LMIC into the community, which should open up options of more invasive changes to the code.

[hallard]

@matthijskooijman
Thanks for your answer. I could do the job to expose API, no problem.
Any idea when you'll be able to merge your mjs branch ?
Once merged I'll do a PR for #24 and I'll add SPI stuff if you don't have time to do it.

anyway, thanks for this great job

[matthijskooijman]

Any idea when you'll be able to merge your mjs branch ?

Soon™, but probably somewhere in the next two weeks.

[matthijskooijman]

IBM is intending to change the license on the LMIC library to the three-clause BSD license. To simplify incorporating your contribution upstream on the next LMIC release, would you be willing to license it under the same license? If so, please explicitly state this in a comment to this PR? Saying something like "This contribution is licensed under the terms of the three-clause BSD license, as linked above" should be sufficient.

[frankleonrose]

This contribution is licensed under the terms of the three-clause BSD license, as linked above.

[hallard]

This contribution is licensed under the terms of the three-clause BSD license, as linked above

[matthijskooijman]

This repository is now deprecated, see #297 for some more background. I'm grateful for your contribution, but it will no longer be merged. I'm recommending people to use the MCCI version of LMIC instead. If this PR addresses an issue that also exists in that version, I would encourage you to resubmit your contribution there, so it might benefit other users. I'm sorry for the inconvenience...