Re: PWM API, new features should be driver specific or not?

[markus]

wow - it's an email day for me 

I like the idea of having a single pointer for the additional values, or 
changing `data` to a pointer to said structure instead of a `void*` (and move 
the void* into that structure).

Personally I probably would have combined the ISRs into a single callback 
function and use an additional enumeration parameter to determine the 
"trigger". This would allow future extensions, say if a counter supports more 
than one compare value.


As for the nrf52 suggestion, what I meant was that the new api is a super-set 
of the old api. So once you implement a driver with the new api you can easily 
implement a driver with the old api by creating an interface layer that just 
sets all extra pointers and values to dummy values.

Have fun,
Markus


PS: about all the many emails - I got the STM32 port to work yesterday so 
before going further I wanted to clarify the interface, and then you sent out 
this email - and I didn't want to mix topics, hijack your thread . So if 
anybody asks, I'll blame you - just so you know ;)


On Thu, 1 Mar 2018 01:54:53 -0300
Miguel Azevedo  wrote:

> Hi again Markus,
> 
> > I think your enhancements should be their own driver. Plain PWM is
> > such a basic need and feature that I think the interface for it
> > should be as simple as possible. I'm also concerned about the
> > memory footprint increase, which is 3x that of the original
> > interface (and that's just the data structure).  
> 
> Agreed. Although if you re-use the structure it isn't such a dramatic
> increase in memory footprint.
> Regarding the datastructure size I believe we can keep the previous
> interface, without the callback associated fields and the cycle(I
> don't love this one either) I suggest we have a specific datastructure
> defined on pwm_nrf52.h or even pwm.h(given these are common features
> on PWM devices, any driver would be eligible to implement them on a
> standardized way). This struct could either be a struct containing a
> pwm_chan_cfg which we would 'upcast' and 'downcast' (like we do with
> dev and odev) or simply something we pass through the data field.
> 
> > For the nrf52 specifically you could implement the old interface
> > driver on top of a driver with the new proposed interface.  
> I don't understand what you mean by using the old interface but I
> think one of the the ways I described will sort out the pwm_chan_cfg
> memory footprint problem.
> 
> Thanks for your feedback! :)
> 
> Miguel
> 
> On Wed, Feb 28, 2018 at 9:24 PM, markus  wrote:
> > Hi Miguel,
> >
> > I think your enhancements should be their own driver. Plain PWM is
> > such a basic need and feature that I think the interface for it
> > should be as simple as possible. I'm also concerned about the
> > memory footprint increase, which is 3x that of the original
> > interface (and that's just the data structure).
> >
> > For the nrf52 specifically you could implement the old interface
> > driver on top of a driver with the new proposed interface.
> >
> > My 2 cents,
> > Markus
> >
> >
> >
> > On Wed, 28 Feb 2018 17:51:50 -0300
> > Miguel Azevedo  wrote:
> >  
> >> Hi everyone,
> >>
> >> I recently implemented a few new features for the nRF52 PWM driver
> >> and will probably implement them for soft PWM as well.
> >>
> >> The features:
> >> * Interrupts fired every cycle with user handler and parameter
> >> data;
> >> * A mode where the device plays for n cycles;
> >> * Interrupts fired at the end of a sequence of n cycles with user
> >> handler and parameter data.
> >>
> >> These changes are PRed here:
> >> https://github.com/apache/mynewt-core/pull/836
> >>
> >> There is an example (pwm_test) which uses the also PRed easing
> >> library: https://github.com/apache/mynewt-core/pull/822
> >>
> >> These features change the pwm_chan_cfg datastructure on pwm.h,
> >> wich is general to all drivers.
> >> I can argue that these features are implementable on virtually any
> >> PWM device since every hardware PWM implementation I've seen so
> >> far uses timers and counters, however there might be (and probably
> >> are) exceptions.
> >> Anyway a driver which doesn't use these new fields on this
> >> datastructure is still implementable(well, we have soft PWM using
> >> the sabe API interface and it works).
> >>
> >> The alternative to this would be to write a datastructure on the
> >> driver's implementation header file (pwm_nrf52.h in this case) with
> >> the new fields.
> >>
> >> What do you think?
> >>
> >> Best,
> >>
> >> Miguel Azevedo  
> >  
> 
> 
> 

Re: JSON Encoding and Decoding

[Aditya Xavier]

Hi Mynewt Team,

I have been trying to decode JSON using the provided json decoding library.

This particular library seems quite similar to mJSON, however there are 
difference regarding json_read_object etc.

Also unlike mJSON, there is a requirement to write few more methods :-

char jbuf_read_next(struct json_buffer *jb);
char jbuf_read_prev(struct json_buffer *jb);
int jbuf_readn(struct json_buffer *jb, char *buf, int size);
int write(void *buf, char* data, int len);
void buf_init(struct jbuf *ptjb, char *string);

At least thats what I gathered as per the test example. (Am I correct in this 
thought ?)

However, am unable to get the following code working.
/*---
#define MAXCHANNELS 72

long long int PRN[MAXCHANNELS];
long long int elevation[MAXCHANNELS];
long long int azimuth[MAXCHANNELS];

int visible;
struct jbuf tjb;

const struct json_attr_t sat_attrs[] = {
{   .attribute = "PRN",
.type = t_integer,
.addr.integer = PRN
},
{   .attribute = "el", 
.type = t_integer,
.addr.integer = elevation
},
{
.attribute = "az", 
.type = t_integer,
.addr.integer = azimuth
},
{NULL},
};

const struct json_attr_t json_attrs_sky[] = {
{   
.attribute = "class",
.type = t_check,
.dflt.check = "SKY"
},
{
.attribute = "satellites", 
.type = t_array,
.addr.array = {
.element_type = t_structobject,
.arr.objects.subtype=sat_attrs,
.maxlen = MAXCHANNELS,
.count = 
}
},
{NULL},
};

int fetch_map(const char *map){

//  int i;

buf_init(,(char *) map);
console_printf("Buffer Initiated\n");

json_read_object(_buf, json_attrs_sky);

console_printf("JSON Read %d!\n", visible);

// for (i = 0; i < visible; i++){
console_printf("PRN = %lld, elevation = %lld, azimuth = %lld\n", 
PRN[0], elevation[0], azimuth[0]);
console_printf("PRN = %lld, elevation = %lld, azimuth = %lld\n", 
PRN[1], elevation[1], azimuth[1]);
// }
printf(“Completed\n");
return 1;

}
*/

Calling this method using 

fetch_map(R"=({"class":"SKY","satellites":[{"PRN":10,"el":45,"az":196,"used":true},{"PRN":29,"el":67,"az":310,"used":true}]})=“);

Am getting the following response :-

01 Buffer Initiated
02 JSON Read 0!
03 PRN = 10, elevation = 45, azimuth = 196
04 PRN = 0, elevation = 0, azimuth = 0
Completed

Which means its not populating the count value in the array as it should. 

It is also quite possible am completely wrong about it :)

Thanks,
Aditya Xavier.


> On 28-Feb-2018, at 10:49 PM, Aditya Xavier  wrote:
> 
> I have been trying to use the same example as is; assuming that apache was 
> also utilizing mJSON.
> 
> However, it seems there are quite some differences from that library. For 
> e.g. 
> 
> .addr.integer = PRN  is no longer valid.
> 
> It is now 
> .arr.integers.store = PRN
> 
> And json_read_object requires a struct json_buffer *jb instead of a char 
> array.
> 
> This struct has to be populated using user method etc. There are multiple 
> differences from the MicroJSON implementation.
> 
> Isn’t there some good documentation on this ?
> 
> 
>> On 28-Feb-2018, at 10:39 PM, Christopher Collins  wrote:
>> 
>> Hi Aditya,
>> 
>> On Wed, Feb 28, 2018 at 06:35:04PM +0530, Aditya Xavier wrote:
>>> Thanks got Encoding working for the required JSON.
>>> 
>>> Any pointers on how to get the decoding working ?
>>> 
>>> From the example 
>>> https://github.com/apache/mynewt-core/blob/master/encoding/json/test/src/testcases/json_simple_decode.c
>>>  
>>> 
>>> 
>>> Am able to decode upto name2 only.
>>> 
>>> {“name1": 1,”name2": “value2”, “name3”: [{“name4”: 2, “name5”: 3}]};
>>> 
>>> Please let me know the Structure for Array of objects.
>> 
>> Example 2 in the microjson document
>> (http://www.catb.org/~esr/microjson/microjson.html) does something
>> similar, so I would take a look at that.  This example is under the
>> "Compound Value Types" heading.
>> 
>> Chris
> 

Re: Device numbering clarification

[markus]

Hey Chris,

your description matches what I see in the code.

While working on the STM32F3 port I was using that port for a sample 
application to get ppl started on controlling their robots with an Android 
device. And I noticed that depending on which part I was working on (STM32F3 
port, bsp or app) I changed my mind - or better my perspective.

What I came up with was this (sticking with the uart example):

-) the device name (eg "uart0") belongs to the application. As an application 
developer, if the board supports at least on serial port it will be named 
"uart0". If there are 2 serial ports they will be available as "uart0" and 
"uart1"  This is the same as on a desktop or notebook computer, the device 
numbering starts at "0" and increments with each available device.

-) the sycfg name (eg "UART_0") belongs to the BSP - and doesn't necessarily 
have a relationship to the device of a /similar/ name (aka the same number). 
The reason I ended up here is because the board I was using has one HW uart 
connected to the USB bridge. It seems that most (all?) bsps use "UART_0" for 
this connection - and so did I.

When I was at the point to switch from the test setup to use the UART which was 
connected to the BT module I didn't want to change the application and realised 
that the BSP can declare multiple devices and one can use syscfg to select the 
one uart that matches the app - assuming that whichever one of the two uarts is 
enabled shows up as "uart0".

This setup allows the BSP to support a lot of devices/device configurations 
without any ill consequences on memory footprint and code size (unless all of 
those device are actually enabled). But again, the numbering should start at 0 
and increase by 1 for each additional device.

Given the discussion a few weeks ago about moving the device declarations into 
the mcu, this should probably move there and only leave the device creation 
depending on the syscfg values in the BSP. Haven't figured out how that should 
look like though - not there yet.

-) the mcu HW peripheral name belongs to the mcu and I don't think there is any 
value in correlating that to any of the numbers above. For quite a few mcus it 
wouldn't even be possible because the mcy peripheral numbering starts at `1` 
whereas all mynewt devices start at `0`.

Not the most important topic to think about, I just wanted to make sure I'm not 
violating some sacred standard.

Thanks a bunch,
Markus

On Wed, 28 Feb 2018 11:36:35 -0800
Christopher Collins  wrote:

> Hi Markus,
> 
> On Mon, Feb 26, 2018 at 11:36:41AM -0800, markus wrote:
> > Is there some documentation about the numbers, meanings and
> > relationship (if any) of the 3 different device numbers?
> > 
> >  -) syscfg, eg UART_0
> >  -) device name, eg "uart0"
> >  -) mcu HW device, eg "USART2"  
> 
> The short answer is: unfortunately no, there isn't any documentation
> about how devices should be numbered.  I think the device numbering
> scheme is something that evolved without any concrete plan, so a
> conversation might be a good idea.
> 
> For the first two (syscfg and os_dev name): these should always
> correspond to the same device.  I.e., `UART_0` syscfg settings should
> configure the device called "uart0".
> 
> The third item (name that the MCU datasheet assigned to a peripheral)
> is not so straightforward.  I think there is some tension due to two
> conflicting goals:
> 1. Make setting names consistent with names in MCU documentation.
> 2. Make setting names consistent among all BSPs.
> 
> The second goal seems to have won out over the first.  The reason the
> second goal is important is that it allows for BSP-agnostic libraries
> and apps.  For example, a library that requires a UART can be
> configured to use a UART number without worrying about whether the MCU
> documentation calls the device "UART 0" or "USART 0".
> 
> If you have any thoughts on how this could be improved, please don't
> hesitate to share them.  The same goes for other readers!
> 
> Chris

Re: clarification about pwm_test

[markus]

Hey Miguel,

thanks for the background that helps a lot.

I agree that (if this is the way the pwm api should work) the top value should 
be explicit. As the return value from pwm_set_frequency would be an option - 
given its prominence though it would probably make sense to add an API call for 
it or add it as a member to `struct pwm_dev`.

At least that way the dependency between the application and driver 
implementation gets broken.

It would be great if we could nail that down here so I can implement the stm 
driver correctly the first time ;).

The pin-less PWM is more a result of calling the APIs in the wrong order. I 
couldn't come up with a use case but also wasn't sure if it should be 
supported. I do have robot though where the h-bridge is a bit hard to control. 
What I ended up doing was to generate the same PWM in 2 hw-units and depending 
on state enabled one of the output pins.

Good stuff,
Markus


On Thu, 1 Mar 2018 00:48:36 -0300
Miguel Azevedo  wrote:

> Hi Markus,
> 
> > According to the doc the duty cycle should be absolutely defined
> > between 0 (no output) and 65535 (full output).  
> Yes, the doc on the PWM API wasn't updated after the first drivers got
> implemented, if you take a look at pwm_nrf52 on my latest pull request
> on PWM the driver's doc is different.
> 
> > Just to be clear, the way I understand the interface a call like
> >
> >   pwm_enable_duty_cycle(pwm, 0, 0x7FFF);
> >
> > should always lead to a 50% duty cycle, regardless of what the pwm
> > frequency was set to and regardless of what counter frequency is
> > internally used by the pwm driver.  
> 
> Well, in a general way PWM uses clock division, so in practice you
> have a top value, which is clock_freq/pwm_freq and this should be the
> 100% duty cycle.
> Some versions ago there was a PWM HAL API, I based the current PWM API
> on that one, with a few differences which BTW where discussed on this
> mailing list.
> Why not convert it and always have a 0 - 65535 and not have external
> code doing it?
> At the time I implemented the pwm_nrf52 and soft_pwm drivers it made
> more sense to have the duty cycle fraction value based on the top
> value. This is because it would totally respect the resolution and
> remove the need to convert from one value range to another each time
> we change duty cycle.
> If you convert it internally every time you change the duty cycle,
> which probably will happen frequently, it won't be as efficient as
> calculating a top value once you set the frequency(something which is
> likely to happen less frequently).
> 
> > I would appreciate any clarifying comments as to what's going on
> > here - or where I'm going wrong.  
> What happens here is that I forgot to change the documentation,
> although the drivers work consistently the way I just described.
> Just to be clear here: I understand your concern about the fraction
> range and that it is on the older API.
> It still makes sense to me to use the real range and calculate the top
> value when we set the frequency rather than converting from one value
> range to another every time we change the duty cycle, its just the way
> PWM works. Probably it would be better to return a top value on the
> set_frequency function, this is one of the changes would like to
> propose.
> 
> > The second question has to do with the sequence of api calls and at
> > what point the PWM is actually started (which is an explicit step
> > for STM32s).
> > - Is it a safe assumption that an application must call
> > pwm_chan_config for a channel before it can call
> > pwm_enable_duty_cycle?  
> Absolutely. As far as we go we need to set a GPIO output before we
> fire up a PWM channel.
> 
> > - Is there support for kicking off a PWM channel without
> > configuring an output pin?  
> No, can you give us a use case where that would need happen?
> 
> > Lastly, in some email archive about the original PWM driver thread
> > I found a reference to 'hal_pwm' - which I can't even find a header
> > for. Did this get removed from the repo?  
> Yes, it did, here you have it:
> https://github.com/apache/mynewt-core/blob/mynewt_0_9_0_tag/hw/hal/include/hal/hal_pwm.h
> 
> Best,
> 
> Miguel
> 
> On Wed, Feb 28, 2018 at 9:53 PM, markus  wrote:
> > I started implementing a PWM driver for the STM32 processors a few
> > days ago and ran into an issue I need clarification. According to
> > the doc the duty cycle should be absolutely defined between 0 (no
> > output) and 65535 (full output).
> >
> > However, pwm_test and pwm_nrf52 require the application code to get
> > the base frequency, calculate the top value from that and then
> > scale the duty cycle accordingly.
> >
> > Just to be clear, the way I understand the interface a call like
> >
> >   pwm_enable_duty_cycle(pwm, 0, 0x7FFF);
> >
> > should always lead to a 50% duty cycle, regardless of what the pwm
> > frequency was set to and regardless of what counter frequency is
> > internally 

Re: recommendations for IO pin declarations

[Miguel Azevedo]

Hi markus,

> `struct pwm_chan_config` only specifies the pin number. I've been using the 
> `data` member to put the burden on the application to specify the "correct" 
> number for the alternate function > - but I find this a little unsatisfactory.
That sounds a lot driver/hw specific. Can you show us the code for that?

Thanks,
Miguel

On Wed, Feb 28, 2018 at 9:57 PM, markus  wrote:
> While writing a PWM driver for the STM32s mcu family I came across the issue 
> of assigning output pins for a pwm channel.
>
> PWM output in STM32s is done by configuring a pin for "alternate functions" - 
> where specific pins have specific (possible) alternate functions. In other 
> words, only a very limited number of timer:channel:pin combinations are 
> possible, each one requiring a very specific alternate function.
>
> `struct pwm_chan_config` only specifies the pin number. I've been using the 
> `data` member to put the burden on the application to specify the "correct" 
> number for the alternate function - but I find this a little unsatisfactory.
>
> So I was wondering if somebody has a recommendation, better approach to 
> solving this issue?
>
> Have fun,
> Markus



-- 
--
Miguel Azevedo

Re: PWM API, new features should be driver specific or not?

[Miguel Azevedo]

Hi again Markus,

> I think your enhancements should be their own driver. Plain PWM is such a 
> basic need and feature that I think the interface for it should be as simple 
> as possible. I'm also concerned about the memory footprint increase, which is 
> 3x that of the original interface (and that's just the data structure).

Agreed. Although if you re-use the structure it isn't such a dramatic
increase in memory footprint.
Regarding the datastructure size I believe we can keep the previous
interface, without the callback associated fields and the cycle(I
don't love this one either) I suggest we have a specific datastructure
defined on pwm_nrf52.h or even pwm.h(given these are common features
on PWM devices, any driver would be eligible to implement them on a
standardized way). This struct could either be a struct containing a
pwm_chan_cfg which we would 'upcast' and 'downcast' (like we do with
dev and odev) or simply something we pass through the data field.

> For the nrf52 specifically you could implement the old interface driver on 
> top of a driver with the new proposed interface.
I don't understand what you mean by using the old interface but I
think one of the the ways I described will sort out the pwm_chan_cfg
memory footprint problem.

Thanks for your feedback! :)

Miguel

On Wed, Feb 28, 2018 at 9:24 PM, markus  wrote:
> Hi Miguel,
>
> I think your enhancements should be their own driver. Plain PWM is such a 
> basic need and feature that I think the interface for it should be as simple 
> as possible. I'm also concerned about the memory footprint increase, which is 
> 3x that of the original interface (and that's just the data structure).
>
> For the nrf52 specifically you could implement the old interface driver on 
> top of a driver with the new proposed interface.
>
> My 2 cents,
> Markus
>
>
>
> On Wed, 28 Feb 2018 17:51:50 -0300
> Miguel Azevedo  wrote:
>
>> Hi everyone,
>>
>> I recently implemented a few new features for the nRF52 PWM driver and
>> will probably implement them for soft PWM as well.
>>
>> The features:
>> * Interrupts fired every cycle with user handler and parameter data;
>> * A mode where the device plays for n cycles;
>> * Interrupts fired at the end of a sequence of n cycles with user
>> handler and parameter data.
>>
>> These changes are PRed here:
>> https://github.com/apache/mynewt-core/pull/836
>>
>> There is an example (pwm_test) which uses the also PRed easing
>> library: https://github.com/apache/mynewt-core/pull/822
>>
>> These features change the pwm_chan_cfg datastructure on pwm.h, wich is
>> general to all drivers.
>> I can argue that these features are implementable on virtually any PWM
>> device since every hardware PWM implementation I've seen so far uses
>> timers and counters, however there might be (and probably are)
>> exceptions.
>> Anyway a driver which doesn't use these new fields on this
>> datastructure is still implementable(well, we have soft PWM using the
>> sabe API interface and it works).
>>
>> The alternative to this would be to write a datastructure on the
>> driver's implementation header file (pwm_nrf52.h in this case) with
>> the new fields.
>>
>> What do you think?
>>
>> Best,
>>
>> Miguel Azevedo
>



-- 
--
Miguel Azevedo

Re: clarification about pwm_test

[Miguel Azevedo]

Hi Markus,

> According to the doc the duty cycle should be absolutely defined between 0 
> (no output) and 65535 (full output).
Yes, the doc on the PWM API wasn't updated after the first drivers got
implemented, if you take a look at pwm_nrf52 on my latest pull request
on PWM the driver's doc is different.

> Just to be clear, the way I understand the interface a call like
>
>   pwm_enable_duty_cycle(pwm, 0, 0x7FFF);
>
> should always lead to a 50% duty cycle, regardless of what the pwm frequency 
> was set to and regardless of what counter frequency is internally used by the 
> pwm driver.

Well, in a general way PWM uses clock division, so in practice you
have a top value, which is clock_freq/pwm_freq and this should be the
100% duty cycle.
Some versions ago there was a PWM HAL API, I based the current PWM API
on that one, with a few differences which BTW where discussed on this
mailing list.
Why not convert it and always have a 0 - 65535 and not have external
code doing it?
At the time I implemented the pwm_nrf52 and soft_pwm drivers it made
more sense to have the duty cycle fraction value based on the top
value. This is because it would totally respect the resolution and
remove the need to convert from one value range to another each time
we change duty cycle.
If you convert it internally every time you change the duty cycle,
which probably will happen frequently, it won't be as efficient as
calculating a top value once you set the frequency(something which is
likely to happen less frequently).

> I would appreciate any clarifying comments as to what's going on here - or 
> where I'm going wrong.
What happens here is that I forgot to change the documentation,
although the drivers work consistently the way I just described.
Just to be clear here: I understand your concern about the fraction
range and that it is on the older API.
It still makes sense to me to use the real range and calculate the top
value when we set the frequency rather than converting from one value
range to another every time we change the duty cycle, its just the way
PWM works. Probably it would be better to return a top value on the
set_frequency function, this is one of the changes would like to
propose.

> The second question has to do with the sequence of api calls and at what 
> point the PWM is actually started (which is an explicit step for STM32s).
> - Is it a safe assumption that an application must call pwm_chan_config for a 
> channel before it can call pwm_enable_duty_cycle?
Absolutely. As far as we go we need to set a GPIO output before we
fire up a PWM channel.

> - Is there support for kicking off a PWM channel without configuring an 
> output pin?
No, can you give us a use case where that would need happen?

> Lastly, in some email archive about the original PWM driver thread I found a 
> reference to 'hal_pwm' - which I can't even find a header for. Did this get 
> removed from the repo?
Yes, it did, here you have it:
https://github.com/apache/mynewt-core/blob/mynewt_0_9_0_tag/hw/hal/include/hal/hal_pwm.h

Best,

Miguel

On Wed, Feb 28, 2018 at 9:53 PM, markus  wrote:
> I started implementing a PWM driver for the STM32 processors a few days ago 
> and ran into an issue I need clarification. According to the doc the duty 
> cycle should be absolutely defined between 0 (no output) and 65535 (full 
> output).
>
> However, pwm_test and pwm_nrf52 require the application code to get the base 
> frequency, calculate the top value from that and then scale the duty cycle 
> accordingly.
>
> Just to be clear, the way I understand the interface a call like
>
>   pwm_enable_duty_cycle(pwm, 0, 0x7FFF);
>
> should always lead to a 50% duty cycle, regardless of what the pwm frequency 
> was set to and regardless of what counter frequency is internally used by the 
> pwm driver.
>
> I would appreciate any clarifying comments as to what's going on here - or 
> where I'm going wrong.
>
> The second question has to do with the sequence of api calls and at what 
> point the PWM is actually started (which is an explicit step for STM32s).
> - Is it a safe assumption that an application must call pwm_chan_config for a 
> channel before it can call pwm_enable_duty_cycle?
> - Is there support for kicking off a PWM channel without configuring an 
> output pin?
>
> Lastly, in some email archive about the original PWM driver thread I found a 
> reference to 'hal_pwm' - which I can't even find a header for. Did this get 
> removed from the repo?
>
> Thanks a lot,
> Markus
>
>
>



-- 
--
Miguel Azevedo

recommendations for IO pin declarations

[markus]

While writing a PWM driver for the STM32s mcu family I came across the issue of 
assigning output pins for a pwm channel.

PWM output in STM32s is done by configuring a pin for "alternate functions" - 
where specific pins have specific (possible) alternate functions. In other 
words, only a very limited number of timer:channel:pin combinations are 
possible, each one requiring a very specific alternate function.

`struct pwm_chan_config` only specifies the pin number. I've been using the 
`data` member to put the burden on the application to specify the "correct" 
number for the alternate function - but I find this a little unsatisfactory.

So I was wondering if somebody has a recommendation, better approach to solving 
this issue?

Have fun,
Markus

clarification about pwm_test

[markus]

I started implementing a PWM driver for the STM32 processors a few days ago and 
ran into an issue I need clarification. According to the doc the duty cycle 
should be absolutely defined between 0 (no output) and 65535 (full output).

However, pwm_test and pwm_nrf52 require the application code to get the base 
frequency, calculate the top value from that and then scale the duty cycle 
accordingly.

Just to be clear, the way I understand the interface a call like

  pwm_enable_duty_cycle(pwm, 0, 0x7FFF);

should always lead to a 50% duty cycle, regardless of what the pwm frequency 
was set to and regardless of what counter frequency is internally used by the 
pwm driver.

I would appreciate any clarifying comments as to what's going on here - or 
where I'm going wrong.

The second question has to do with the sequence of api calls and at what point 
the PWM is actually started (which is an explicit step for STM32s).
- Is it a safe assumption that an application must call pwm_chan_config for a 
channel before it can call pwm_enable_duty_cycle?
- Is there support for kicking off a PWM channel without configuring an output 
pin?

Lastly, in some email archive about the original PWM driver thread I found a 
reference to 'hal_pwm' - which I can't even find a header for. Did this get 
removed from the repo?

Thanks a lot,
Markus

Re: PWM API, new features should be driver specific or not?

[markus]

Hi Miguel,

I think your enhancements should be their own driver. Plain PWM is such a basic 
need and feature that I think the interface for it should be as simple as 
possible. I'm also concerned about the memory footprint increase, which is 3x 
that of the original interface (and that's just the data structure).

For the nrf52 specifically you could implement the old interface driver on top 
of a driver with the new proposed interface.

My 2 cents,
Markus



On Wed, 28 Feb 2018 17:51:50 -0300
Miguel Azevedo  wrote:

> Hi everyone,
> 
> I recently implemented a few new features for the nRF52 PWM driver and
> will probably implement them for soft PWM as well.
> 
> The features:
> * Interrupts fired every cycle with user handler and parameter data;
> * A mode where the device plays for n cycles;
> * Interrupts fired at the end of a sequence of n cycles with user
> handler and parameter data.
> 
> These changes are PRed here:
> https://github.com/apache/mynewt-core/pull/836
> 
> There is an example (pwm_test) which uses the also PRed easing
> library: https://github.com/apache/mynewt-core/pull/822
> 
> These features change the pwm_chan_cfg datastructure on pwm.h, wich is
> general to all drivers.
> I can argue that these features are implementable on virtually any PWM
> device since every hardware PWM implementation I've seen so far uses
> timers and counters, however there might be (and probably are)
> exceptions.
> Anyway a driver which doesn't use these new fields on this
> datastructure is still implementable(well, we have soft PWM using the
> sabe API interface and it works).
> 
> The alternative to this would be to write a datastructure on the
> driver's implementation header file (pwm_nrf52.h in this case) with
> the new fields.
> 
> What do you think?
> 
> Best,
> 
> Miguel Azevedo

PWM API, new features should be driver specific or not?

[Miguel Azevedo]

Hi everyone,

I recently implemented a few new features for the nRF52 PWM driver and
will probably implement them for soft PWM as well.

The features:
* Interrupts fired every cycle with user handler and parameter data;
* A mode where the device plays for n cycles;
* Interrupts fired at the end of a sequence of n cycles with user
handler and parameter data.

These changes are PRed here:
https://github.com/apache/mynewt-core/pull/836

There is an example (pwm_test) which uses the also PRed easing library:
https://github.com/apache/mynewt-core/pull/822

These features change the pwm_chan_cfg datastructure on pwm.h, wich is
general to all drivers.
I can argue that these features are implementable on virtually any PWM
device since every hardware PWM implementation I've seen so far uses
timers and counters, however there might be (and probably are)
exceptions.
Anyway a driver which doesn't use these new fields on this
datastructure is still implementable(well, we have soft PWM using the
sabe API interface and it works).

The alternative to this would be to write a datastructure on the
driver's implementation header file (pwm_nrf52.h in this case) with
the new fields.

What do you think?

Best,

Miguel Azevedo

Re: Device numbering clarification

[Christopher Collins]

Hi Markus,

On Mon, Feb 26, 2018 at 11:36:41AM -0800, markus wrote:
> Is there some documentation about the numbers, meanings and relationship (if 
> any) of the 3 different device numbers?
> 
>  -) syscfg, eg UART_0
>  -) device name, eg "uart0"
>  -) mcu HW device, eg "USART2"

The short answer is: unfortunately no, there isn't any documentation
about how devices should be numbered.  I think the device numbering
scheme is something that evolved without any concrete plan, so a
conversation might be a good idea.

For the first two (syscfg and os_dev name): these should always
correspond to the same device.  I.e., `UART_0` syscfg settings should
configure the device called "uart0".

The third item (name that the MCU datasheet assigned to a peripheral) is
not so straightforward.  I think there is some tension due to two
conflicting goals:
1. Make setting names consistent with names in MCU documentation.
2. Make setting names consistent among all BSPs.

The second goal seems to have won out over the first.  The reason the
second goal is important is that it allows for BSP-agnostic libraries
and apps.  For example, a library that requires a UART can be configured
to use a UART number without worrying about whether the MCU
documentation calls the device "UART 0" or "USART 0".

If you have any thoughts on how this could be improved, please don't
hesitate to share them.  The same goes for other readers!

Chris

Re: JSON Encoding and Decoding

[Aditya Xavier]

I have been trying to use the same example as is; assuming that apache was also 
utilizing mJSON.

However, it seems there are quite some differences from that library. For e.g. 

.addr.integer = PRN  is no longer valid.

It is now 
.arr.integers.store = PRN

And json_read_object requires a struct json_buffer *jb instead of a char array.

This struct has to be populated using user method etc. There are multiple 
differences from the MicroJSON implementation.

Isn’t there some good documentation on this ?


> On 28-Feb-2018, at 10:39 PM, Christopher Collins  wrote:
> 
> Hi Aditya,
> 
> On Wed, Feb 28, 2018 at 06:35:04PM +0530, Aditya Xavier wrote:
>> Thanks got Encoding working for the required JSON.
>> 
>> Any pointers on how to get the decoding working ?
>> 
>> From the example 
>> https://github.com/apache/mynewt-core/blob/master/encoding/json/test/src/testcases/json_simple_decode.c
>>  
>> 
>> 
>> Am able to decode upto name2 only.
>> 
>> {“name1": 1,”name2": “value2”, “name3”: [{“name4”: 2, “name5”: 3}]};
>> 
>> Please let me know the Structure for Array of objects.
> 
> Example 2 in the microjson document
> (http://www.catb.org/~esr/microjson/microjson.html) does something
> similar, so I would take a look at that.  This example is under the
> "Compound Value Types" heading.
> 
> Chris

Re: JSON Encoding and Decoding

[Christopher Collins]

Hi Aditya,

On Wed, Feb 28, 2018 at 06:35:04PM +0530, Aditya Xavier wrote:
> Thanks got Encoding working for the required JSON.
> 
> Any pointers on how to get the decoding working ?
> 
> From the example 
> https://github.com/apache/mynewt-core/blob/master/encoding/json/test/src/testcases/json_simple_decode.c
>  
> 
> 
> Am able to decode upto name2 only.
> 
> {“name1": 1,”name2": “value2”, “name3”: [{“name4”: 2, “name5”: 3}]};
> 
> Please let me know the Structure for Array of objects.

Example 2 in the microjson document
(http://www.catb.org/~esr/microjson/microjson.html) does something
similar, so I would take a look at that.  This example is under the
"Compound Value Types" heading.

Chris

Re: JSON Encoding and Decoding

[Aditya Xavier]

Thanks got Encoding working for the required JSON.

Any pointers on how to get the decoding working ?

From the example 
https://github.com/apache/mynewt-core/blob/master/encoding/json/test/src/testcases/json_simple_decode.c
 


Am able to decode upto name2 only.

{“name1": 1,”name2": “value2”, “name3”: [{“name4”: 2, “name5”: 3}]};

Please let me know the Structure for Array of objects.


struct json_attr_t test_attr[3] = {
[0] = {
.attribute = “name1",
.type = t_integer,
.addr.integer = _val,
.nodefault = true
},
[1] = {
.attribute = “name2",
.type = t_string,
.addr.string = string1,
.nodefault = true,
.len = sizeof(string1)
},
[2] = {
.attribute = “name3",
.type = t_array,
.addr.array = {
.element_type = t_integer,
.arr.integers.store = intarr,
.maxlen = sizeof intarr / sizeof intarr[0],
.count = _count,
},
.nodefault = true,
.len = sizeof(intarr)
},
[6] = {
.attribute = NULL
}
};


> On 28-Feb-2018, at 2:05 PM, marko kiiskila  wrote:
> 
> 
> 
>> On Feb 28, 2018, at 10:10 AM, Aditya Xavier  wrote:
>> 
>> Yes, that was derp from my end.
>> 
>> Any clue on how to encode the JSON structure I mentioned ?
>> 
>> Is this method correct ?
> 
> You need to initialize the encoder; it needs to be told where to write
> the encoded data. Specifically, you need to at least fill in je_write
> function pointer. This is who’ll get the stream of encoded data.
> 
> Take a look at unit test @ 
> encoding/json/test/src/testcases/json_simple_encode.c
> 
> Otherwise it looks pretty good, based on my cursory inspection.
> 
>> 
>>  struct json_encoder *encoder, *module;
>>  struct json_value data;
>> 
>>  memset(, 0, sizeof(encoder));
>>  memset(, 0, sizeof(module));
>> 
>>  json_encode_object_start(encoder);
>> 
>>  JSON_VALUE_INT(, 1);
>>  json_encode_object_entry(encoder, “name1", );
>> 
>>  JSON_VALUE_STRING(, “value2");
>>  json_encode_object_entry(encoder, “name2", );
>> 
>>  json_encode_array_name(encoder, “name3");
>>  json_encode_array_start(encoder);
>> 
>>  if(getType){
>>  json_encode_object_start(module);
>>  
>>  JSON_VALUE_INT(, 4);
>>  json_encode_object_entry(module, “name4", );
>> 
>>  JSON_VALUE_INT(, 5);
>>  json_encode_object_entry(module, “name5", );
>>  
>>  json_encode_object_finish(module);
>>  }
>>  
>>  json_encode_array_finish(encoder);
>>  json_encode_object_finish(encoder);
>> 
>> 
>> 
>>> On 28-Feb-2018, at 1:18 PM, marko kiiskila  wrote:
>>> 
>>> Hi Aditya,
>>> 
 On Feb 28, 2018, at 9:26 AM, Aditya Xavier  wrote:
 
 HI Mynewt Team,
 
Wanted some assistance on how to encode and decode the following JSON 
 string.
 
{“name1":1,”name2”:"value2”,"name3":[{“name4":1,”name5":5}]}
 
Because of sparse documentation, I used the Test sample; however am 
 stuck with encoding an array of objects.
 
>>> 
>>> …
>>> 
 
I followed the example provided at 
 https://mynewt.apache.org/latest/os/modules/json/json_encode_object_entry 
 
 
And it gives me error :-
error: implicit declaration of function 'json_encode_object_start'
 
 Thanks,
 Aditya Xavier.
>>> 
>>> you need to #include  to see function/macro declarations.
>>> 
>> 
> 

Re: JSON Encoding and Decoding

[marko kiiskila]

> On Feb 28, 2018, at 10:10 AM, Aditya Xavier  wrote:
> 
> Yes, that was derp from my end.
> 
> Any clue on how to encode the JSON structure I mentioned ?
> 
> Is this method correct ?

You need to initialize the encoder; it needs to be told where to write
the encoded data. Specifically, you need to at least fill in je_write
function pointer. This is who’ll get the stream of encoded data.

Take a look at unit test @ encoding/json/test/src/testcases/json_simple_encode.c

Otherwise it looks pretty good, based on my cursory inspection.

> 
>   struct json_encoder *encoder, *module;
>   struct json_value data;
> 
>   memset(, 0, sizeof(encoder));
>   memset(, 0, sizeof(module));
> 
>   json_encode_object_start(encoder);
> 
>   JSON_VALUE_INT(, 1);
>   json_encode_object_entry(encoder, “name1", );
> 
>   JSON_VALUE_STRING(, “value2");
>   json_encode_object_entry(encoder, “name2", );
> 
>   json_encode_array_name(encoder, “name3");
>   json_encode_array_start(encoder);
> 
>   if(getType){
>   json_encode_object_start(module);
>   
>   JSON_VALUE_INT(, 4);
>   json_encode_object_entry(module, “name4", );
> 
>   JSON_VALUE_INT(, 5);
>   json_encode_object_entry(module, “name5", );
>   
>   json_encode_object_finish(module);
>   }
>   
>   json_encode_array_finish(encoder);
>   json_encode_object_finish(encoder);
> 
> 
> 
>> On 28-Feb-2018, at 1:18 PM, marko kiiskila  wrote:
>> 
>> Hi Aditya,
>> 
>>> On Feb 28, 2018, at 9:26 AM, Aditya Xavier  wrote:
>>> 
>>> HI Mynewt Team,
>>> 
>>> Wanted some assistance on how to encode and decode the following JSON 
>>> string.
>>> 
>>> {“name1":1,”name2”:"value2”,"name3":[{“name4":1,”name5":5}]}
>>> 
>>> Because of sparse documentation, I used the Test sample; however am 
>>> stuck with encoding an array of objects.
>>> 
>> 
>> …
>> 
>>> 
>>> I followed the example provided at 
>>> https://mynewt.apache.org/latest/os/modules/json/json_encode_object_entry 
>>> 
>>> 
>>> And it gives me error :-
>>> error: implicit declaration of function 'json_encode_object_start'
>>> 
>>> Thanks,
>>> Aditya Xavier.
>> 
>> you need to #include  to see function/macro declarations.
>> 
> 

Re: JSON Encoding and Decoding

[Aditya Xavier]

Yes, that was derp from my end.

Any clue on how to encode the JSON structure I mentioned ?

Is this method correct ?

struct json_encoder *encoder, *module;
struct json_value data;

memset(, 0, sizeof(encoder));
memset(, 0, sizeof(module));

json_encode_object_start(encoder);

JSON_VALUE_INT(, 1);
json_encode_object_entry(encoder, “name1", );

JSON_VALUE_STRING(, “value2");
json_encode_object_entry(encoder, “name2", );

json_encode_array_name(encoder, “name3");
json_encode_array_start(encoder);

if(getType){
json_encode_object_start(module);

JSON_VALUE_INT(, 4);
json_encode_object_entry(module, “name4", );

JSON_VALUE_INT(, 5);
json_encode_object_entry(module, “name5", );

json_encode_object_finish(module);
}

json_encode_array_finish(encoder);
json_encode_object_finish(encoder);



> On 28-Feb-2018, at 1:18 PM, marko kiiskila  wrote:
> 
> Hi Aditya,
> 
>> On Feb 28, 2018, at 9:26 AM, Aditya Xavier  wrote:
>> 
>> HI Mynewt Team,
>> 
>>  Wanted some assistance on how to encode and decode the following JSON 
>> string.
>> 
>>  {“name1":1,”name2”:"value2”,"name3":[{“name4":1,”name5":5}]}
>> 
>>  Because of sparse documentation, I used the Test sample; however am 
>> stuck with encoding an array of objects.
>> 
> 
> …
> 
>> 
>>  I followed the example provided at 
>> https://mynewt.apache.org/latest/os/modules/json/json_encode_object_entry 
>> 
>> 
>>  And it gives me error :-
>>  error: implicit declaration of function 'json_encode_object_start'
>> 
>> Thanks,
>> Aditya Xavier.
> 
> you need to #include  to see function/macro declarations.
>