Closed matthijskooijman closed 4 years ago
ricaun
commented
4 years ago My board has the DIO3 floating and works normal, but when the code tries to set SetDIO3AsTcxoCtrl the module freak out and stop working. I tried to put the voltage to 0 but does not work. 😞
A hidden parameter on the struct lmic_pinmap could disable the DIO3-as TCXO-control, I think it is better than create an external command to disable this feature.
matthijskooijman
commented
4 years ago Ah, Thomas Telkamp suggested that enabling DIO3 for TCXO-control also puts the transceiver into external oscillator mode (which needs no driving), rather than crystal mode (which have to be driven).
A hidden parameter on the struct lmic_pinmap could disable the DIO3-as TCXO-control, I think it is better than create an external command to disable this feature.
One problem of the current pinmap approach is that, IIRC, optional parameters have only limited support: Missing parameters are always zero, and I think missing parameters might also trigger a warning, which is not so nice.
Calling functions during setup could help, but that would of course have extra overhead over a (PROGMEM) struct that is initialized once like the pinmap now. So I'm still not sure how to do this configuration in an efficient and extendable way...
ricaun
commented
4 years ago One problem of the current pinmap approach is that, IIRC, optional parameters have only limited support: Missing parameters are always zero, and I think missing parameters might also trigger a warning, which is not so nice.
The first time I was playing with the library I had some problem, I was not filling all the lmic pinmap parameters I assumed the default value was LMIC_UNUSED_PIN,
It's possible to set the default as LMIC_UNUSED_PIN?
Calling functions during setup could help, but that would of course have extra overhead over a (PROGMEM) struct that is initialized once like the pinmap now. So I'm still not sure how to do this configuration in an efficient and extendable way...
Maybe use the same lmic_pins.tcxo to control the DIO3 for TCXO-control, with another const like.
const u1_t LMIC_DIO3_CONTROL_TCXO = 0xfe;The lmic_pins.tcxo variable is not used on the SX126x anyway.
Add some filter on:
I just look at the arduino part, maybe need to change others part on the hal.cpp of the stm32.
It's better than create a new variable, what do you think?
matthijskooijman
commented
4 years ago It's possible to set the default as LMIC_UNUSED_PIN?
Not as far as I know, it's just C that defines that missing values are 0. In C++, you could maybe define default values for fields in classes, but I think that only works when using constructors, not "designated initializers" (the .nss = ... syntax), and constructors do not have named arguments...
The lmic_pins.tcxo variable is not used on the SX126x anyway.
I think it is, it's just used by hal.cpp? Or are you saying the SX126x radio driver does not call the tcxo pin control hal function?
Maybe use the same lmic_pins.tcxo to control the DIO3 for TCXO-control, with another const like.
That's actually not a bad idea, it's semantically meaningful as well. The same thing could be done for using DIO2 for antenna-switching.
I just look at the arduino part, maybe need to change others part on the hal.cpp of the stm32.
Arduino is the most tricky one to configure, since it has to be done at runtime. The stm32 HAL can just use a define or set a variable directly maybe.
matthijskooijman
commented
4 years ago @ricaun, I fixed this in #17. Care to review that PR and test it on your hardware? I only have TCXO-enabled SX126x boards here.
matthijskooijman
commented
4 years ago Hm, I thought about this a bit more over the evening, and I wonder if instead of assigning txco as LMIC_CONTROLLED_BY_DIO3, it wouldn't be nicer to assign dio3 as LMIC_CONTROLS_TXCO. I thought about this after wondering how to implement this for DIO2 controlling the RXTX switch (should you assign rx or tx as LMIC_CONTROLLED_BY_DIO2. What if you need both manual and automatic control?). Similarly, this could allow both automatic and manual control of the TXCO (though I guess you would mostly use this to enable DIO3-controls-TXCO in the transceiver so it goes into TXCO-mode, while DIO3 is really just floating and the real TXCO is controlled by the MCU, so then you could also just do this automatically when txco is assigned to any other pin value).
I was already quite convinced that this would be nicer on the longer run, but then realized that:
dio3 member, only a dio array (and assigning one array element to this special value is not so nicely readable), anddio only has 3 elements, for DIO0, DIO1 and DIO2. Even though the SX126x does not have a DIO0, so you could interpret this as DIO1, DIO2, DIO3, but that would probably be even more confusing.So, maybe it is better as it is now, and I should just think a bit on whether the DIO2 control logically maps to the TX or RX control and also implement that.
matthijskooijman
commented
4 years ago @ricaun, I went ahead with merging #17, since I wanted to continue with some other things that depend on it. Feedback is still welcome if you can find some time for that. Note that DIO2-handling has also changed, see the list of breaking changes near the top of the README.
ricaun
commented
4 years ago Hello @matthijskooijman I goona make some tests and then give a feedback. 😄
Right now, DIO3 is always configured as a TCXO-enable output, since the modules I used had that (and if DIO3 is floating, it should not hurt). Ideally, this should be optional, and the startup time and output voltage can also be configured by the application or board definition. Also, #5 mentioned problems with DIO3 configured this way.